On VC4, the TV margins on the HDMI connector are implemented by scaling
the planes.
However, if only the TV margins or the connector are changed by a new
state, the planes ending up on that connector won't be. Thus, they won't
be updated properly and we'll effectively ignore that change until the
next commit affecting these planes.
Let's make sure to add all the planes attached to the connector so that
we can update them properly.
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 12a00d644b61..0eafaf0b76e5 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -522,6 +522,22 @@ static int vc4_hdmi_connector_atomic_check(struct drm_connector *connector,
if (!crtc)
return 0;
+ if (old_state->tv.margins.left != new_state->tv.margins.left ||
+ old_state->tv.margins.right != new_state->tv.margins.right ||
+ old_state->tv.margins.top != new_state->tv.margins.top ||
+ old_state->tv.margins.bottom != new_state->tv.margins.bottom) {
+ struct drm_crtc_state *crtc_state;
+ int ret;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
+
+ ret = drm_atomic_add_affected_planes(state, crtc);
+ if (ret)
+ return ret;
+ }
+
if (old_state->colorspace != new_state->colorspace ||
!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
struct drm_crtc_state *crtc_state;
--
2.38.1
Hi
Am 07.12.22 um 17:07 schrieb Maxime Ripard:
> On VC4, the TV margins on the HDMI connector are implemented by scaling
> the planes.
>
> However, if only the TV margins or the connector are changed by a new
> state, the planes ending up on that connector won't be. Thus, they won't
> be updated properly and we'll effectively ignore that change until the
> next commit affecting these planes.
>
> Let's make sure to add all the planes attached to the connector so that
> we can update them properly.
>
> Signed-off-by: Maxime Ripard <[email protected]>
Reviewed-by: Thomas Zimmermann <[email protected]>
> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 12a00d644b61..0eafaf0b76e5 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -522,6 +522,22 @@ static int vc4_hdmi_connector_atomic_check(struct drm_connector *connector,
> if (!crtc)
> return 0;
>
> + if (old_state->tv.margins.left != new_state->tv.margins.left ||
> + old_state->tv.margins.right != new_state->tv.margins.right ||
> + old_state->tv.margins.top != new_state->tv.margins.top ||
> + old_state->tv.margins.bottom != new_state->tv.margins.bottom) {
> + struct drm_crtc_state *crtc_state;
> + int ret;
> +
> + crtc_state = drm_atomic_get_crtc_state(state, crtc);
> + if (IS_ERR(crtc_state))
> + return PTR_ERR(crtc_state);
> +
> + ret = drm_atomic_add_affected_planes(state, crtc);
This is slightly dangerous, but works in the given case. I'd appreciate
a comment ala 'plane state will be checked by atomic helpers'.
The plane state of the added planes has to be tested via their
atomic_check helpers. That's no problem here because the connector's
atomic_check runs before the plane's atomic_check. See _check_modeset vs
_check_planes in [1].
We had code that invoked drm_atomic_add_affected_planes() from the
CRTC's atomic_check. At that point, the plane's atomic_check has been
executed already and the newly added planes were never really state-checked.
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/v6.1.4/source/drivers/gpu/drm/drm_atomic_helper.c#L1069
> + if (ret)
> + return ret;
> + }
> +
> if (old_state->colorspace != new_state->colorspace ||
> !drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
> struct drm_crtc_state *crtc_state;
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev