Re: [PATCH] drm: lcdif: Use adjusted_mode .clock instead of .crtc_clock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/9/24 11:55 AM, Isaac Scott wrote:
On Tue, 2024-10-08 at 23:48 +0200, Marek Vasut wrote:
On 10/8/24 12:07 PM, Isaac Scott wrote:
On Mon, 2024-10-07 at 20:06 +0200, Marek Vasut wrote:
On 10/7/24 7:01 PM, Isaac Scott wrote:
Hi Marek,

Hi,

On Sat, 2024-07-06 at 02:16 +0200, Marek Vasut wrote:
On 6/24/24 11:19 AM, Alexander Stein wrote:
Am Freitag, 31. Mai 2024, 22:27:21 CEST schrieb Marek
Vasut:
In case an upstream bridge modified the required clock
frequency
in its .atomic_check callback by setting
adjusted_mode.clock
,
make sure that clock frequency is generated by the
LCDIFv3
block.

This is useful e.g. when LCDIFv3 feeds DSIM which feeds
TC358767
with (e)DP output, where the TC358767 expects precise
timing
on
its input side, the precise timing must be generated by
the
LCDIF.

Signed-off-by: Marek Vasut <marex@xxxxxxx>

With the other rc358767 patches in place, this does the
trick.
Reviewed-by: Alexander Stein
<alexander.stein@xxxxxxxxxxxxxxx>

I'll pick this up next week if there is no objection.

Unfortunately, this has caused a regression that is present in
v6.12-
rc1 on the i.MX8MP PHYTEC Pollux using the
arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts.
The
display is the edt,etml1010g3dra panel, as per the upstream
dts. We
bisected to this commit, and reverting this change fixed the
screen.

We then tried to retest this on top of v6.12-rc2, and found we
also
had
to revert commit ff06ea04e4cf3ba2f025024776e83bfbdfa05155
("clk:
imx:
clk-imx8mp: Allow media_disp pixel clock reconfigure parent
rate")
alongside this. Reverting these two commits makes the display
work
again at -rc2.

Do you have any suggestions on anything we might be missing on
our
end?
Please let me know if there's anything you'd like me to test as
I'm
not
sure what the underlying fault was here.
I believe what is going on is that the LCDIF cannot configure its
upstream clock because something else is already using those
clock
and
it set those clock to a specific frequency. LCDIF is now trying
to
configure those clock to match the LVDS panel, and it fails, so
it
tries
to set some approximate clock and that is not good enough for the
LVDS
panel.

Can you share dump of /sys/kernel/debug/clk/clk_summary on
failing
and
working system ? You might see the difference around the "video"
clock.

(I have seen this behavior before, the fix was usually a matter
of
moving one of the LCDIFs to another upstream clock like PLL3, so
it
can
pick well matching output clock instead of some horrid
approximation
which then drives the panel likely out of specification)

Hi Marek,

Please find attached the clk_summary for v6.12-rc2 before and after
the
reversion (the one after the reversion is 6.12-
rc2_summary_postfix).
Thank you, this helped greatly.

I believe I know why it used to kind-of work for you, but I'm afraid
this used to work by sheer chance and it does not really work
correctly
for the panel you use, even if the panel likely does show the correct
content. But, there is a way to make it work properly for the panel
you use.

First of all, the pixel clock never really matched the panel-simple.c
pixel clock for the edt_etml1010g3dra_timing:

$ grep '\<media_disp2_pix\>' 6.12-rc2_summary_postfix
    media_disp2_pix  1  1  0  74250000 ...
                              ^^^^^^^^

$ grep -A 1 edt_etml1010g3dra_timing drivers/gpu/drm/panel/panel-
simple.c
static const struct display_timing edt_etml1010g3dra_timing = {
          .pixelclock = { 66300000, 72400000, 78900000 },
                                    ^^^^^^^^

The pixel clock are within tolerance, but there is a discrepancy
74250000 != 72400000 .

Since commit 94e6197dadc9 ("arm64: dts: imx8mp: Add LCDIF2 & LDB
nodes")
the IMX8MP_VIDEO_PLL1_OUT is set to a very specific frequency of
1039500000 Hz, which tidily divides by 2 to 519750000 Hz (which is
your
LVDS serializer frequency) and divides by 7 to 74250000 Hz which is
your
LCDIF pixel clock.

This Video PLL1 configuration since moved to &media_blk_ctrl {} , but
it
is still in the imx8mp.dtsi . Therefore, to make your panel work at
the
correct desired pixel clock frequency instead of some random one
inherited from imx8mp.dtsi, add the following to the pollux DT, I
believe that will fix the problem and is the correct fix:

&media_blk_ctrl {
     // 506800000 = 72400000 * 7 (for single-link LVDS, this is
enough)
     // there is no need to multiply the clock by * 2
     assigned-clock-rates = <500000000>, <200000000>, <0>, <0>,
<500000000>, <506800000>;
};

Can you please test whether this works and the pixel clock are
accurate
in /sys/kernel/debug/clk/clk_summary ?

Interestingly, after making the change you suggested to imx8mp-
phyboard-pollux-rdk.dts before the two reversions, the display now
seems to work. Please see below for the relevant section of the new
clk_summary referring to media_disp2_pix:

video_pll1_ref_sel               1       1        0        24000000
0          0     50000      Y      deviceless
no_connection_id
        video_pll1                    1       1        0
506800000   0          0     50000      Y         deviceless
no_connection_
           video_pll1_bypass          1       1        0
506800000   0          0     50000      Y            deviceless
no_connecti
              video_pll1_out          2       2        0
506800000   0          0     50000      Y               deviceless
no_conne
                 media_ldb            1       1        0
506800000   0          0     50000      Y                  deviceless
no_co
                    media_ldb_root_clk 1       1        0
506800000   0          0     50000      Y
32ec0000.blk-ctrl:bridge@5c     l
deviceless no
                 media_disp2_pix      1       1        0        72400000
0          0     50000      Y                  deviceless
no_co
                    media_disp2_pix_root_clk 1       1        0
72400000    0          0     50000      Y
32e90000.display-controller
32ec0000.blk-ctrl di deviceless no
                 media_disp1_pix      0       0        0
506800000   0          0     50000      N                  deviceless
no_co
                    media_disp1_pix_root_clk 0       0        0
506800000   0          0     50000      N
32ec0000.blk-ctrl
deviceless no
                 media_mipi_phy1_ref  0       0        0        23036364
0          0     50000      N                  deviceless
no_co
                    media_mipi_phy1_ref_root 0       0        0
23036364    0          0     50000      N
32ec0000.blk-ctrl

The media_disp2_pix clock now seems to be correct at 724000000 after
your changes.

Do you want to submit the DT patch with correct Fixes: tag ? :)



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux