Re: [PATCH V2 5/6] drm: bridge: samsung-dsim: Support non-burst mode

[Adam Ford]

On Fri, Apr 28, 2023 at 7:31 AM Marek Szyprowski
<m.szyprowski@xxxxxxxxxxx> wrote:
>
> On 24.04.2023 12:00, Adam Ford wrote:
> > On Mon, Apr 24, 2023 at 3:25 AM Marek Szyprowski
> > <m.szyprowski@xxxxxxxxxxx> wrote:
> >> On 23.04.2023 14:12, Adam Ford wrote:
> >>> The high-speed clock is hard-coded to the burst-clock
> >>> frequency specified in the device tree.  However, when
> >>> using devices like certain bridge chips without burst mode
> >>> and varying resolutions and refresh rates, it may be
> >>> necessary to set the high-speed clock dynamically based
> >>> on the desired pixel clock for the connected device.
> >>>
> >>> This also removes the need to set a clock speed from
> >>> the device tree for non-burst mode operation, since the
> >>> pixel clock rate is the rate requested from the attached
> >>> device like an HDMI bridge chip.  This should have no
> >>> impact for people using burst-mode and setting the burst
> >>> clock rate is still required for those users.
> >>>
> >>> Signed-off-by: Adam Ford <aford173@xxxxxxxxx>
> >> This one breaks Exynos-5433 based TM2e board with a DSI panel.
> > Marek S,
> >
> > Thank you for testing!  I knoiw there are several of us who appreciate
> > your testing this since it's hard to know if something broke without
> > hardware.  Is there any way you can tell me if the flag is set to
> > enable MIPI_DSI_MODE_VIDEO_BURST?
>
> TM2e board uses the DSI panel operated in command mode and handled by
> panel-samsung-s6e3ha2.c driver. The MIPI_DSI_MODE_VIDEO_BURST flag is
> not set by the driver. However, the MIPI_DSI_CLOCK_NON_CONTINUOUS flags
> is set there. I really have no idea if setting VIDEO_BURST would make
> sense together with CLOCK_NON_CONTINUOUS or not. Maybe the driver lacks
> setting it?
>
>
> > I was trying to be diligent about not breaking your boards, but
> > without your boards, it's difficult.  The theory was that if
> > MIPI_DSI_MODE_VIDEO_BURST is set and there is a burst clock set in the
> > device tree, it would use the burst clock.
> >
> > As a fall-back I could just simply check for the presence of the
> > burst_clock_rate instead of both MIPI_DSI_MODE_VIDEO_BURST and
> > burst_clock_rate.
>
> Maybe you should extend your check also for the
> MIPI_DSI_CLOCK_NON_CONTINUOUS flag? Does it make sense?

Looking at some of the devices that might attach in the future, It
appears that ti-sn65dsi86.c sets this flag.  It's a display port
bridge, so I would expect it to need a variable clock rate similar to
how the HDMI bridge that I need works.  I am concerned that I make the
burst clock dependent on MIPI_DSI_CLOCK_NON_CONTINUOUS, it might break
the Display Port bridge.

I think it's better to just check if the samsung,burst-clock-frequency
is present in the device tree and use it when present.  If it's not
present, then fall back to the pixel clock of the connected device.

I looked at a bunch of Exynos parts, and it looks like they all use
the samsung,burst-clock-frequency device tree setting.  Is that true,
or did I miss one?

adam
>
>  > ...
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>