[PATCH v2] drm: xlnx: zynqmp_dpsub: Enable plane in atomic update

[PATCH v2] drm: xlnx: zynqmp_dpsub: Enable plane in atomic update

[Anatoliy Klymenko]

Unconditionally enable the DPSUB layer in the corresponding atomic plane
update callback. Setting the new display mode may require disabling and
re-enabling the CRTC. This effectively resets DPSUB to the default state
with all layers disabled. The original implementation of the plane atomic
update enables the corresponding DPSUB layer only if the framebuffer
format has changed. This would leave the layer disabled after switching to
a different display mode with the same framebuffer format.

Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
---
Changes in v2:
- Added comment around DPSUB layer enablement explaining why it should be
  done unconditionally.
- Link to v1: https://lore.kernel.org/r/20240520-dp-layer-enable-v1-1-c9b481209115@amd.com
---
 drivers/gpu/drm/xlnx/zynqmp_kms.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
index 43bf416b33d5..0b57ab5451a9 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
@@ -120,9 +120,13 @@ static void zynqmp_dpsub_plane_atomic_update(struct drm_plane *plane,
 		zynqmp_disp_blend_set_global_alpha(dpsub->disp, true,
 						   plane->state->alpha >> 8);
 
-	/* Enable or re-enable the plane if the format has changed. */
-	if (format_changed)
-		zynqmp_disp_layer_enable(layer);
+	/*
+	 * Unconditionally enable the layer, as it may have been disabled
+	 * previously either explicitly to reconfigure layer format, or
+	 * implicitly after DPSUB reset during display mode change. DRM
+	 * framework calls this callback for enabled planes only.
+	 */
+	zynqmp_disp_layer_enable(layer);
 }
 
 static const struct drm_plane_helper_funcs zynqmp_dpsub_plane_helper_funcs = {

---
base-commit: 673087d8b023faf34b84e8faf63bbeea3da87bab
change-id: 20240520-dp-layer-enable-7b561af29ca8

Best regards,
-- 
Anatoliy Klymenko <anatoliy.klymenko@amd.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] drm: xlnx: zynqmp_dpsub: Enable plane in atomic update

[Tomi Valkeinen]

Hi,

On 24/05/2024 02:49, Anatoliy Klymenko wrote:
> Unconditionally enable the DPSUB layer in the corresponding atomic plane
> update callback. Setting the new display mode may require disabling and
> re-enabling the CRTC. This effectively resets DPSUB to the default state
> with all layers disabled. The original implementation of the plane atomic
> update enables the corresponding DPSUB layer only if the framebuffer
> format has changed. This would leave the layer disabled after switching to
> a different display mode with the same framebuffer format.
> 
> Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
> ---
> Changes in v2:
> - Added comment around DPSUB layer enablement explaining why it should be
>    done unconditionally.
> - Link to v1: https://lore.kernel.org/r/20240520-dp-layer-enable-v1-1-c9b481209115@amd.com
> ---
>   drivers/gpu/drm/xlnx/zynqmp_kms.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> index 43bf416b33d5..0b57ab5451a9 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> @@ -120,9 +120,13 @@ static void zynqmp_dpsub_plane_atomic_update(struct drm_plane *plane,
>   		zynqmp_disp_blend_set_global_alpha(dpsub->disp, true,
>   						   plane->state->alpha >> 8);
>   
> -	/* Enable or re-enable the plane if the format has changed. */
> -	if (format_changed)
> -		zynqmp_disp_layer_enable(layer);
> +	/*
> +	 * Unconditionally enable the layer, as it may have been disabled
> +	 * previously either explicitly to reconfigure layer format, or
> +	 * implicitly after DPSUB reset during display mode change. DRM
> +	 * framework calls this callback for enabled planes only.
> +	 */
> +	zynqmp_disp_layer_enable(layer);
>   }
>   
>   static const struct drm_plane_helper_funcs zynqmp_dpsub_plane_helper_funcs = {
> 
> ---
> base-commit: 673087d8b023faf34b84e8faf63bbeea3da87bab
> change-id: 20240520-dp-layer-enable-7b561af29ca8
> 
> Best regards,

Thanks, I have pushed this to drm-misc-next.

  Tomi

RE: [PATCH v2] drm: xlnx: zynqmp_dpsub: Enable plane in atomic update

[Klymenko, Anatoliy]

> -----Original Message-----
> From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Sent: Monday, June 17, 2024 12:44 AM
> To: Klymenko, Anatoliy <Anatoliy.Klymenko@amd.com>; Laurent Pinchart
> <laurent.pinchart@ideasonboard.com>; Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>; Maxime Ripard
> <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>;
> David Airlie <airlied@gmail.com>; Daniel Vetter <daniel@ffwll.ch>;
> Simek, Michal <michal.simek@amd.com>
> Cc: dri-devel@lists.freedesktop.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2] drm: xlnx: zynqmp_dpsub: Enable plane in atomic
> update
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> Hi,
> 
> On 24/05/2024 02:49, Anatoliy Klymenko wrote:
> > Unconditionally enable the DPSUB layer in the corresponding atomic
> plane
> > update callback. Setting the new display mode may require disabling
> and
> > re-enabling the CRTC. This effectively resets DPSUB to the default state
> > with all layers disabled. The original implementation of the plane
> atomic
> > update enables the corresponding DPSUB layer only if the framebuffer
> > format has changed. This would leave the layer disabled after switching
> to
> > a different display mode with the same framebuffer format.
> >
> > Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
> > ---
> > Changes in v2:
> > - Added comment around DPSUB layer enablement explaining why it
> should be
> >    done unconditionally.
> > - Link to v1: https://lore.kernel.org/r/20240520-dp-layer-enable-v1-1-
> c9b481209115@amd.com
> > ---
> >   drivers/gpu/drm/xlnx/zynqmp_kms.c | 10 +++++++---
> >   1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> > index 43bf416b33d5..0b57ab5451a9 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> > @@ -120,9 +120,13 @@ static void
> zynqmp_dpsub_plane_atomic_update(struct drm_plane *plane,
> >               zynqmp_disp_blend_set_global_alpha(dpsub->disp, true,
> >                                                  plane->state->alpha >> 8);
> >
> > -     /* Enable or re-enable the plane if the format has changed. */
> > -     if (format_changed)
> > -             zynqmp_disp_layer_enable(layer);
> > +     /*
> > +      * Unconditionally enable the layer, as it may have been disabled
> > +      * previously either explicitly to reconfigure layer format, or
> > +      * implicitly after DPSUB reset during display mode change. DRM
> > +      * framework calls this callback for enabled planes only.
> > +      */
> > +     zynqmp_disp_layer_enable(layer);
> >   }
> >
> >   static const struct drm_plane_helper_funcs
> zynqmp_dpsub_plane_helper_funcs = {
> >
> > ---
> > base-commit: 673087d8b023faf34b84e8faf63bbeea3da87bab
> > change-id: 20240520-dp-layer-enable-7b561af29ca8
> >
> > Best regards,
> 
> Thanks, I have pushed this to drm-misc-next.
> 
>   Tomi

Thank you,
Anatoliy