On Fri, Apr 28, 2023 at 9:04 AM Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote: > > On 28.04.2023 15:35, Adam Ford wrote: > > 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. > > Right, this sounds rational. > > > 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? > > That true. All Exynos based boards with DSI panels use constant DSI > burst frequency defined in the device tree. Thanks for the feedback. I'll try to get another rev of this series pushed later today or Monday. adam > > > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland >
