In order to test the current atomic_check hooks we need to have a DRM
device that has roughly the same capabilities and layout that the actual
hardware. We'll also need a bunch of functions to create arbitrary
atomic states.
Let's create some helpers to create a device that behaves like the real
one, and some helpers to maintain the atomic state we want to check.
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Maíra Canal <mcanal@igalia.com>
Link: https://lore.kernel.org/r/20221123-rpi-kunit-tests-v3-17-4615a663a84a@cerno.tech
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Commit f0601ef863 ("drm/vc4: vec: Protect device resources after
removal") add fail path for vc4_vec_encoder_enable(), and will put
usage_counter only when pm_runtime_get_sync() succeeds. However,
pm_runtime_get_sync() will increment usage_counter even it failed. Fix
it by replacing it with pm_runtime_resume_and_get() to keep usage
counter balanced.
Fixes: e4b81f8c74 ("drm/vc4: Add support for the VEC (Video Encoder) IP")
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
Link: https://lore.kernel.org/r/20221124015113.18540-1-shangxiaojing@huawei.com
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
On the vc4 devices (and later), the blending is done by a single device
called the HVS. The HVS has three FIFO that can operate in parallel, and
route their output to 6 CRTCs and 7 encoders on the BCM2711.
Each of these CRTCs and encoders have some constraints on which FIFO
they can feed from, so we need some code to take all those constraints
into account and assign FIFOs to CRTCs.
The problem can be simplified by assigning those FIFOs to CRTCs by
ascending output index number. We had a comment mentioning it already,
but we were never actually enforcing it.
It was working still in most situations because the probe order is
roughly equivalent, except for the (optional, and fairly rarely used on
the Pi4) VEC which was last in the probe order sequence, but one of the
earliest device to assign.
This resulted in configurations that were rejected by our code but were
still valid with a different assignment.
We can fix this by making sure we assign CRTCs to FIFOs by ordering
them by ascending HVS output index.
Fixes: 87ebcd42fb ("drm/vc4: crtc: Assign output to channel automatically")
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20221123-rpi-kunit-tests-v1-10-051a0bb60a16@cerno.tech
Add support for the following composite output modes (all of them are
somewhat more obscure than the previously defined ones):
- NTSC_443 - NTSC-style signal with the chroma subcarrier shifted to
4.43361875 MHz (the PAL subcarrier frequency). Never used for
broadcasting, but sometimes used as a hack to play NTSC content in PAL
regions (e.g. on VCRs).
- PAL_N - PAL with alternative chroma subcarrier frequency,
3.58205625 MHz. Used as a broadcast standard in Argentina, Paraguay
and Uruguay to fit 576i50 with colour in 6 MHz channel raster.
- PAL60 - 480i60 signal with PAL-style color at normal European PAL
frequency. Another non-standard, non-broadcast mode, used in similar
contexts as NTSC_443. Some displays support one but not the other.
- SECAM - French frequency-modulated analog color standard; also have
been broadcast in Eastern Europe and various parts of Africa and Asia.
Uses the same 576i50 timings as PAL.
Also added some comments explaining color subcarrier frequency
registers.
Acked-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Mateusz Kwiatkowski <kfyatek+publicgit@gmail.com>
Tested-by: Mateusz Kwiatkowski <kfyatek+publicgit@gmail.com>
Acked-in-principle-or-something-like-that-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://lore.kernel.org/r/20220728-rpi-analog-tv-properties-v10-18-256dad125326@cerno.tech
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Now that the core can deal fine with analog TV modes, let's convert the vc4
VEC driver to leverage those new features.
We've added some backward compatibility to support the old TV mode property
and translate it into the new TV norm property. We're also making use of
the new analog TV atomic_check helper to make sure we trigger a modeset
whenever the TV mode is updated.
Acked-by: Noralf Trønnes <noralf@tronnes.org>
Tested-by: Mateusz Kwiatkowski <kfyatek+publicgit@gmail.com>
Acked-in-principle-or-something-like-that-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://lore.kernel.org/r/20220728-rpi-analog-tv-properties-v10-17-256dad125326@cerno.tech
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
The drm_atomic_get_new_private_obj_state() function returns NULL
on error path, drm_atomic_get_old_private_obj_state() function
returns NULL on error path, too, they does not return error pointers.
By the way, vc4_hvs_get_new/old_global_state() should return
ERR_PTR(-EINVAL), otherwise there will be null-ptr-defer issue,
such as follows:
In function vc4_atomic_commit_tail():
|-- old_hvs_state = vc4_hvs_get_old_global_state(state); <-- return NULL
|-- if (WARN_ON(IS_ERR(old_hvs_state))) <-- no return
|-- unsigned long state_rate = max(old_hvs_state->core_clock_rate,
new_hvs_state->core_clock_rate); <-- null-ptr-defer
Fixes: 9ec03d7f1e ("drm/vc4: kms: Wait on previous FIFO users before a commit")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20221110094445.2930509-6-cuigaosheng1@huawei.com
A problem about modprobe vc4 failed is triggered with the following log
given:
[ 420.327987] Error: Driver 'vc4_hvs' is already registered, aborting...
[ 420.333904] failed to register platform driver vc4_hvs_driver [vc4]: -16
modprobe: ERROR: could not insert 'vc4': Device or resource busy
The reason is that vc4_drm_register() returns platform_driver_register()
directly without checking its return value, if platform_driver_register()
fails, it returns without unregistering all the vc4 drivers, resulting the
vc4 can never be installed later.
A simple call graph is shown as below:
vc4_drm_register()
platform_register_drivers() # all vc4 drivers are registered
platform_driver_register()
driver_register()
bus_add_driver()
priv = kzalloc(...) # OOM happened
# return without unregister drivers
Fixing this problem by checking the return value of
platform_driver_register() and do platform_unregister_drivers() if
error happened.
Fixes: c8b75bca92 ("drm/vc4: Add KMS support for Raspberry Pi.")
Signed-off-by: Yuan Can <yuancan@huawei.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20221103014705.109322-1-yuancan@huawei.com
Move the generic fbdev implementation into its own source and header
file. Adapt drivers. No functional changes, but some of the internal
helpers have been renamed to fit into the drm_fbdev_ naming scheme.
v3:
* rename drm_fbdev.{c,h} to drm_fbdev_generic.{c,h}
* rebase onto vmwgfx changes
* rebase onto xlnx changes
* fix include statements in amdgpu
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221103151446.2638-22-tzimmermann@suse.de
drm-misc-next for 6.2:
UAPI Changes:
Cross-subsystem Changes:
- dma-buf: locking improvements
- firmware: New API in the RaspberryPi firmware driver used by vc4
Core Changes:
- client: Null pointer dereference fix in drm_client_buffer_delete()
- mm/buddy: Add back random seed log
- ttm: Convert ttm_resource to use size_t for its size, fix for an
undefined behaviour
Driver Changes:
- bridge:
- adv7511: use dev_err_probe
- it6505: Fix return value check of pm_runtime_get_sync
- panel:
- sitronix: Fixes and clean-ups
- lcdif: Increase DMA burst size
- rockchip: runtime_pm improvements
- vc4: Fix for a regression preventing the use of 4k @ 60Hz, and
further HDMI rate constraints check.
- vmwgfx: Cursor improvements
Signed-off-by: Dave Airlie <airlied@redhat.com>
From: Maxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20221103083437.ksrh3hcdvxaof62l@houat
Commit ae71ab585c ("drm/vc4: hdmi: Enforce the minimum rate at
runtime_resume") reintroduced the call to clk_set_min_rate in an attempt
to fix the boot without a monitor connected on the RaspberryPi3.
However, that introduced a regression breaking the display output
entirely (black screen but no vblank timeout) on the Pi4.
This is due to the fact that we now have in a typical modeset at boot,
in vc4_hdmi_encoder_pre_crtc_configure(), we have a first call to
clk_set_min_rate() asking for the minimum rate of the HSM clock for our
given resolution, and then a call to pm_runtime_resume_and_get(). We
will thus execute vc4_hdmi_runtime_resume() which, since the commit
mentioned above, will call clk_set_min_rate() a second time with the
absolute minimum rate we want to enforce on the HSM clock.
We're thus effectively erasing the minimum mandated by the mode we're
trying to set. The fact that only the Pi4 is affected is due to the fact
that it uses a different clock driver that tries to minimize the HSM
clock at all time. It will thus lower the HSM clock rate to 120MHz on
the second clk_set_min_rate() call.
The Pi3 doesn't use the same driver and will not change the frequency on
the second clk_set_min_rate() call since it's still within the new
boundaries and it doesn't have the code to minimize the clock rate as
needed. So even though the boundaries are still off, the clock rate is
still the right one for our given mode, so everything works.
There is a lot of moving parts, so I couldn't find any obvious
solution:
- Reverting the original is not an option, as that would break the Pi3
again.
- We can't move the clk_set_min_rate() call in _pre_crtc_configure()
since because, on the Pi3, the HSM clock has the CLK_SET_RATE_GATE
flag which prevents the clock rate from being changed after it's
been enabled. Our calls to clk_set_min_rate() can change it, so they
need to be done before clk_prepare_enable().
- We can't remove the call to clk_prepare_enable() from the
runtime_resume hook to put it into _pre_crtc_configure() either,
since we need that clock to be enabled to access the registers, and
we can't count on the fact that the display will be active in all
situations (doing any CEC operation, or listing the modes while
inactive are valid for example()).
- We can't drop the call to clk_set_min_rate() in
_pre_crtc_configure() since we would need to still enforce the
minimum rate for a given resolution, and runtime_resume doesn't have
access to the current mode, if there's any.
- We can't copy the TMDS character rate into vc4_hdmi and reuse it
since, because it's part of the KMS atomic state, it needs to be
protected by a mutex. Unfortunately, some functions (CEC operations,
mostly) can be reentrant (through the CEC framework) and still need
a pm_runtime_get.
However, we can work around this issue by leveraging the fact that the
clk_set_min_rate() calls set boundaries for its given struct clk, and
that each different clk_get() call will return a different instance of
struct clk. The clock framework will then aggregate the boundaries for
each struct clk instances linked to a given clock, plus its hardware
boundaries, and will use that.
We can thus get an extra HSM clock user for runtime_pm use only, and use
our different clock instances depending on the context: runtime_pm will
use its own to set the absolute minimum clock setup so that we never
lock the CPU waiting for a register access, and the modeset part will
set its requirement for the current resolution. And we let the CCF do
the coordination.
It's not an ideal solution, but it's fairly unintrusive and doesn't
really change any part of the logic so it looks like a rather safe fix.
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2136234
Fixes: ae71ab585c ("drm/vc4: hdmi: Enforce the minimum rate at runtime_resume")
Reported-by: Peter Robinson <pbrobinson@gmail.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Peter Robinson <pbrobinson@gmail.com>
Link: https://lore.kernel.org/r/20221021131339.2203291-1-maxime@cerno.tech
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
A comment introduced by commit 6bed2ea3cb ("drm/vc4: hdmi: Reset link
on hotplug") mentions a drm_atomic_helper_connector_hdmi_reset_link()
function that was part of the earlier versions but got moved internally
and is now named vc4_hdmi_reset_link(). Let's fix the function name.
Fixes: 6bed2ea3cb ("drm/vc4: hdmi: Reset link on hotplug")
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Link: https://lore.kernel.org/r/20221024093634.118190-2-maxime@cerno.tech
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
We access some fields protected by our internal mutex in
vc4_hdmi_reset_link() (saved_adjusted_mode, output_bpc, output_format)
and are calling functions that need to have that lock taken
(vc4_hdmi_supports_scrambling()).
However, the current code doesn't lock that mutex. Let's make sure it
does.
Fixes: 6bed2ea3cb ("drm/vc4: hdmi: Reset link on hotplug")
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Link: https://lore.kernel.org/r/20221024093634.118190-1-maxime@cerno.tech
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Following the clock rate range improvements to the clock framework,
trying to set a disjoint range on a clock will now result in an error.
Thus, we can't set a minimum rate higher than the maximum reported by
the firmware, or clk_set_min_rate() will fail.
Thus we need to clamp the rate we are about to ask for to the maximum
rate possible on that clock.
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Link: https://lore.kernel.org/r/20220815-rpi-fix-4k-60-v5-7-fe9e7ac8b111@cerno.tech
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
In order to support higher HDMI frequencies, users have to set the
hdmi_enable_4kp60 parameter in their config.txt file.
This will have the side-effect of raising the maximum of the core clock,
tied to the HVS, and managed by the HVS driver.
However, we are querying this in the HDMI driver by poking into the HVS
structure to get our struct clk handle.
Let's make this part of the HVS bind implementation to have all the core
clock related setup in the same place.
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Link: https://lore.kernel.org/r/20220815-rpi-fix-4k-60-v5-5-fe9e7ac8b111@cerno.tech
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
In order to support higher HDMI frequencies, users have to set the
hdmi_enable_4kp60 parameter in their config.txt file.
We were detecting this so far by calling clk_round_rate() on the core
clock with the frequency we're supposed to run at when one of those
modes is enabled. Whether or not the parameter was enabled could then be
inferred by the returned rate since the maximum clock rate reported by
the firmware was one of the side effect of setting that parameter.
However, the recent clock rework we did changed what clk_round_rate()
was returning to always return the minimum allowed, and thus this test
wasn't reliable anymore.
Let's use the new clk_get_max_rate() function to reliably determine the
maximum rate allowed on that clock and fix the 4k@60Hz output.
Fixes: e9d6cea2af ("clk: bcm: rpi: Run some clocks at the minimum rate allowed")
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Link: https://lore.kernel.org/r/20220815-rpi-fix-4k-60-v5-4-fe9e7ac8b111@cerno.tech
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
drm-misc-next for 6.2:
UAPI Changes:
- Documentation for page-flip flags
Cross-subsystem Changes:
- dma-buf: Add unlocked variant of vmapping and attachment-mapping
functions
Core Changes:
- atomic-helpers: CRTC primary plane test fixes
- connector: TV API consistency improvements, cmdline parsing
improvements
- crtc-helpers: Introduce drm_crtc_helper_atomic_check() helper
- edid: Fixes for HFVSDB parsing,
- fourcc: Addition of the Vivante tiled modifier
- makefile: Sort and reorganize the objects files
- mode_config: Remove fb_base from drm_mode_config_funcs
- sched: Add a module parameter to change the scheduling policy,
refcounting fix for fences
- tests: Sort the Kunit tests in the Makefile, improvements to the
DP-MST tests
- ttm: Remove unnecessary drm_mm_clean() call
Driver Changes:
- New driver: ofdrm
- Move all drivers to a common dma-buf locking convention
- bridge:
- adv7533: Remove dynamic lane switching
- it6505: Runtime PM support
- ps8640: Handle AUX defer messages
- tc358775: Drop soft-reset over I2C
- ast: Atomic Gamma LUT Support, Convert to SHMEM, various
improvements
- lcdif: Support for YUV planes
- mgag200: Fix PLL Setup on some revisions
- udl: Modesetting improvements, hot-unplug support
- vc4: Fix support for PAL-M
Signed-off-by: Dave Airlie <airlied@redhat.com>
From: Maxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20221020072405.g3o4hxuk75gmeumw@houat
This is a revert of commit fd5894fa24 ("drm/vc4: hdmi: Remove clock
rate initialization"), with the code slightly moved around.
It turns out that we can't downright remove that code from the driver,
since the Pi0-3 and Pi4 are in different cases, and it only works for
the Pi4.
Indeed, the commit mentioned above was relying on the RaspberryPi
firmware clocks driver to initialize the rate if it wasn't done by the
firmware. However, the Pi0-3 are using the clk-bcm2835 clock driver that
wasn't doing this initialization. We therefore end up with the clock not
being assigned a rate, and the CPU stalling when trying to access a
register.
We can't move that initialization in the clk-bcm2835 driver, since the
HSM clock we depend on is actually part of the HDMI power domain, so any
rate setup is only valid when the power domain is enabled. Thus, we
reinstated the minimum rate setup at runtime_suspend, which should
address both issues.
Link: https://lore.kernel.org/dri-devel/20220922145448.w3xfywkn5ecak2et@pengutronix.de/
Fixes: fd5894fa24 ("drm/vc4: hdmi: Remove clock rate initialization")
Reported-by: Marc Kleine-Budde <mkl@pengutronix.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20220929-rpi-pi3-unplugged-fixes-v1-1-cd22e962296c@cerno.tech
The VC4 HDMI controller driver relies on the HDMI codec ASoC driver. In
order to set it up properly, in vc4_hdmi_audio_init(), our HDMI driver
will register a device matching the HDMI codec driver, and then register
an ASoC card using that codec.
However, if vc4 is compiled as a module, chances are that the hdmi-codec
driver will be too. In such a case, the module loader will have a very
narrow window to load the module between the device registration and the
card registration.
If it fails to load the module in time, the card registration will fail
with EPROBE_DEFER, and we'll abort the audio initialisation,
unregistering the HDMI codec device in the process.
The next time the bind callback will be run, it's likely that we end up
missing that window again, effectively preventing vc4 to probe entirely.
In order to prevent this, we can create a soft dependency of the vc4
driver on the HDMI codec one so that we're sure the HDMI codec will be
loaded before the VC4 module is, and thus we'll never end up in the
previous situation.
Fixes: 91e99e1139 ("drm/vc4: hdmi: Register HDMI codec")
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20220902144111.3424560-1-maxime@cerno.tech
We currently have two sets of TV properties.
The first one is there to deal with analog TV properties, creating
properties such as the TV mode, subconnectors, saturation, hue and so on.
It's created by calling the drm_mode_create_tv_properties() function.
The second one is there to deal with properties that might be useful on a
TV, creating the overscan margins for example. It's created by calling the
drm_mode_create_tv_margin_properties().
However, we also have a drm_atomic_helper_connector_tv_reset() function
that will reset the TV margin properties to their default values, and thus
is supposed to be called for the latter set. This creates an ambiguity due
to the inconsistent naming.
We can thus rename the drm_atomic_helper_connector_tv_reset() function to
drm_atomic_helper_connector_tv_margins_reset() to remove that ambiguity
and hopefully make it more obvious.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Link: https://lore.kernel.org/r/20220728-rpi-analog-tv-properties-v4-4-60d38873f782@cerno.tech
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
During a hotplug cycle (such as a TV going out of suspend, or when the
cable is disconnected and reconnected), the expectation is that the same
state used before the disconnection is reused until the next commit.
However, the HDMI scrambling requires that some flags are set in the
monitor, and those flags are very likely to be reset when the cable has
been disconnected. This will thus result in a blank display, even if the
display pipeline configuration hasn't been modified or is in the exact
same state.
The solution we've had so far is to enable the scrambling-related bits
again on reconnection, but the HDMI 2.0 specification (Section 6.1.3.1 -
Scrambling Control) requires that the scrambling enable bit is set
before sending any scrambled video signal. Using that solution thus
breaks that expectation.
The solution used by i915 is to do a full modeset on the connector so
that we disable the video signal, enable the scrambling bit, and enable
the video signal again.
As such, we took that code and plugged it into vc4. It probably could
have been turned into an helper, but it proved to be difficult for
several reasons:
* i915 has fairly different structures than simpler KMS drivers such
as vc4, so doing some code that works with both proved to be
difficult;
* Other simpler drivers could reuse some of it (tegra, dw-hdmi), but
it would still require to move some parameters currently stored in
private structure that are needed to compute whether the scrambling
is needed or not, and then inform the driver that it needs to be
enabled. Some of those parameters are already in core structures
(drm_display_mode, drm_display_info, bpc), but the output format
isnt't. Adding it is fairly challenging since unlike the TMDS char
rate or mode, there's no consensus on what format to pick in
drivers, so it's not possible to write some generic code that can
depend on it.
For these reasons, we chose to duplicate the code for now, until someone
else really needs it as well, in which case we will be able to convert
it into a generic helper.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220829134731.213478-8-maxime@cerno.tech
