diff mbox series

[v3,2/5] drm: xlnx: zynqmp_dpsub: Fix timing for live mode

Message ID 20240124025402.373620-3-anatoliy.klymenko@amd.com
State New
Headers
Series Fixing live video input in ZynqMP DPSUB |

Commit Message

Klymenko, Anatoliy Jan. 24, 2024, 2:53 a.m. UTC 
  Expect external video timing in live video input mode, program
DPSUB acordingly.

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
---
 drivers/gpu/drm/xlnx/zynqmp_disp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Laurent Pinchart Feb. 5, 2024, 8:08 a.m. UTC | #1 
Hi Anatoliy,

Thank you for the patch.

On Tue, Jan 23, 2024 at 06:53:59PM -0800, Anatoliy Klymenko wrote:
> Expect external video timing in live video input mode, program
> DPSUB acordingly.

Are there no designs where the DPSUB operates in non-live mode, but uses
a video clock from the PL, for instance to use a different clock
frequency ?

I don't think that use case is very common, so I'm fine with this patch
in order to properly support the more common use case of live input, and
leave the PL clock without live input use case for later.

> 
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> 

No need for a blank line here.

> Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/xlnx/zynqmp_disp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index 407bc07cec69..8a39b3accce5 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -1166,7 +1166,7 @@ void zynqmp_disp_enable(struct zynqmp_disp *disp)
>  	/* Choose clock source based on the DT clock handle. */
>  	zynqmp_disp_avbuf_set_clocks_sources(disp, disp->dpsub->vid_clk_from_ps,
>  					     disp->dpsub->aud_clk_from_ps,
> -					     true);
> +					     disp->dpsub->vid_clk_from_ps);
>  	zynqmp_disp_avbuf_enable_channels(disp);
>  	zynqmp_disp_avbuf_enable_audio(disp);
>
Klymenko, Anatoliy Feb. 5, 2024, 10:22 p.m. UTC | #2 
Hi Laurent,

Thanks a lot for the review.

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Monday, February 5, 2024 12:08 AM
> To: Klymenko, Anatoliy <Anatoliy.Klymenko@amd.com>
> Cc: maarten.lankhorst@linux.intel.com; mripard@kernel.org;
> tzimmermann@suse.de; airlied@gmail.com; daniel@ffwll.ch; Simek, Michal
> <michal.simek@amd.com>; dri-devel@lists.freedesktop.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 2/5] drm: xlnx: zynqmp_dpsub: Fix timing for live mode
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> Hi Anatoliy,
> 
> Thank you for the patch.
> 
> On Tue, Jan 23, 2024 at 06:53:59PM -0800, Anatoliy Klymenko wrote:
> > Expect external video timing in live video input mode, program DPSUB
> > acordingly.
> 
> Are there no designs where the DPSUB operates in non-live mode, but uses a
> video clock from the PL, for instance to use a different clock frequency ?
> 
> I don't think that use case is very common, so I'm fine with this patch in order to
> properly support the more common use case of live input, and leave the PL clock
> without live input use case for later.
> 
Theoretically, we can create such a design, but it wouldn't make too much sense since the PS clock is the best choice here. Probably, in some complicated scenarios like hybrid live-in/DMA or live-out, we would consider using the PL video clock, but the DPSUB driver is not supporting them yet.
> >
> > Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >
> 
> No need for a blank line here.
> 
Sure, I'll fix this in the new version. Thank you.
> > Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > ---
> >  drivers/gpu/drm/xlnx/zynqmp_disp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > index 407bc07cec69..8a39b3accce5 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > @@ -1166,7 +1166,7 @@ void zynqmp_disp_enable(struct zynqmp_disp *disp)
> >       /* Choose clock source based on the DT clock handle. */
> >       zynqmp_disp_avbuf_set_clocks_sources(disp, disp->dpsub-
> >vid_clk_from_ps,
> >                                            disp->dpsub->aud_clk_from_ps,
> > -                                          true);
> > +
> > + disp->dpsub->vid_clk_from_ps);
> >       zynqmp_disp_avbuf_enable_channels(disp);
> >       zynqmp_disp_avbuf_enable_audio(disp);
> >
> 
> --
> Regards,
> 
> Laurent Pinchart
diff mbox series

Patch

diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 407bc07cec69..8a39b3accce5 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -1166,7 +1166,7 @@  void zynqmp_disp_enable(struct zynqmp_disp *disp)
 	/* Choose clock source based on the DT clock handle. */
 	zynqmp_disp_avbuf_set_clocks_sources(disp, disp->dpsub->vid_clk_from_ps,
 					     disp->dpsub->aud_clk_from_ps,
-					     true);
+					     disp->dpsub->vid_clk_from_ps);
 	zynqmp_disp_avbuf_enable_channels(disp);
 	zynqmp_disp_avbuf_enable_audio(disp);