Message-ID: <2d1b404288a6f0b99f26b697df1ff975@vosn.de>
Date: Mon, 09 Dec 2024 09:46:38 +0100
From: Nikolaus Voss <nv@...n.de>
To: Marek Vasut <marex@...x.de>
Cc: Liu Ying <victor.liu@....nxp.com>, Alexander Stein
 <alexander.stein@...tq-group.com>, Liu Ying <victor.liu@....com>, Luca
 Ceresoli <luca.ceresoli@...tlin.com>, Fabio Estevam <festevam@...x.de>,
 Andrzej Hajda <andrzej.hajda@...el.com>, Neil Armstrong
 <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>, Laurent
 Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman
 <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, David Airlie
 <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 nikolaus.voss@...g-streit.com, miquel.raynal@...tlin.com
Subject: Re: [PATCH] drm: bridge: fsl-ldb: fixup mode on freq mismatch

Hi Marek,

On 07.12.2024 12:30, Marek Vasut wrote:
> On 12/4/24 11:55 AM, Nikolaus Voss wrote:
>>>>>>>> I doubt that pixel clock tree cannot find appropriate division 
>>>>>>>> ratios
>>>>>>>> for some pixel clock rates, especially for dual-link LVDS on 
>>>>>>>> i.MX8MP
>>>>>>>> and i.MX93 platforms, because PLL clock rate should be 7x faster 
>>>>>>>> than
>>>>>>>> pixel clock rate and 2x faster than "ldb" clock rate so that the 
>>>>>>>> 3.5
>>>>>>>> folder between "ldb" clock and pixel clock can be met. That 
>>>>>>>> means the
>>>>>>>> PLL clock rate needs to be explicitly set first for this case.
>>>>>>>> 
>>>>>>>> Can you assign the PLL clock rate in DT to satisfy the "ldb" and 
>>>>>>>> pixel
>>>>>>>> clock rates like the below commit does, if you use a LVDS panel?
>>>>>>>> 
>>>>>>>> 4fbb73416b10 ("arm64: dts: imx8mp-phyboard-pollux: Set Video 
>>>>>>>> PLL1
>>>>>>>> frequency to 506.8 MHz")
>>>>>>> 
>>>>>>> I probably could. The point of my patch is you don't have to know 
>>>>>>> in
>>>>>>> advance which LVDS panel is connected, and you don't have to 
>>>>>>> calculate
>>>>>>> the base PLL clock by hand and store it in the device tree.
>>>>>>> 
>>>>>>> In my test system, I have three different LVDS panels with EDID 
>>>>>>> EEPROM,
>>>>>>> none of which worked with the stock driver, but all work with 
>>>>>>> this
>>>>>>> patch.
>>>>>>> With slightly adapted pixel clocks though.
>>>>>> 
>>>>>> If each of the three LVDS panels has only one display mode, you 
>>>>>> may
>>>>>> assign the PLL clock rates in DT overlays for the panels.
>>>>> I temporarily agree.
>>>>> 
>>>>> I also currently use DTOs for various panels including their PLL
>>>>> setting, but in the end, I think/hope the work of Miquel and co. is
>>>>> going to make that PLL setting part unnecessary.
>>>> 
>>>> That is exactly what my patch is about. I want to use one DT for all
>>>> panels
>>> 
>>> Right
>>> 
>>>> and store the panel's timing in EDID EEPROM.
>>> Oh, that is a new one. Does the EDID EEPROM store the entirety of
>>> 'struct display_timing {}' somehow , or is that a custom format ?
>> 
>> Well, sort of ;-). VESA has taken care of this 30 years ago
>> (https://en.wikipedia.org/wiki/Extended_Display_Identification_Data).
>> 
>> DRM handles this with drm_get_edid() and siblings, e.g. :
> 
> EDID can not encode all the information in struct display_timing {} ,
> or can it ?
> 
> I think what you would be missing are bus_flags , bus_format and
> possibly the single/dual link and channel (odd/even) mapping, won't
> you ?

Yes, that's right. I use the vendor block for bus_flags and bus_format
now, but that's not standard and not portable of course.

My first idea was to store the DT overlay in the display EEPROM but
a standard 1k EEPROM is too small for that.

-- 
Nikolaus Voss

Powered by blists - more mailing lists