| Message ID | 20221207-rpi-hdmi-improvements-v1-1-6b15f774c13a@cerno.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vc4: hdmi: Broadcast RGB, BT601, BT2020 | expand |
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 <maxime@cerno.tech> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > 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; >
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;
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 <maxime@cerno.tech> --- drivers/gpu/drm/vc4/vc4_hdmi.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)