Re: [PATCH v4 00/20] drm: starfive: jh7110: Enable display subsystem

[Michal Wilczynski]

Hi Bryan and Icenowy,

On 9/20/26 09:35, Icenowy Zheng wrote:
> 在 2026-09-20日的 02:09 -0400，Byron Stanoszek写道：
>> On Tue, 15 Sep 2026, Michal Wilczynski wrote:
>>
>>> This series enables the display subsystem on the StarFive JH7110.
>>
>> Hi Michal,
>>
>> I tested your v4 patch series on my VisionFive2 1.3B with kernel 7.3-
>> rc3 today
>> and it works very well, except for 2 issues. Note, to make the screen
>> refresh
>> properly, I am using the noncoherent cache fixes from
>> https://protect2.fireeye.com/v1/url?k=3d78ed45-62e3d45a-3d79660a-000babdfecba-dbc6e750c3338bb9&q=1&e=d9589fcd-304c-4065-91c9-c5043969df15&u=https%3A%2F%2Fgithub.com%2Fganboing-jh7110%2Flinux%2Fcommits%2Fganboing-jh7110-cache-fix-v1%2F
>> (specifically commits 3683d97, ef93e2c, 277db0b, and 26ce3e6) in
>> addition to
>> your patch.
>>
>> Issue 1: I have a Dell U2711 monitor which has limited HDMI 1.3
>> capability, so
>> I invented my own Xorg mode to get a good frame rate:
>>
>> "2560x1440_40.22"  162.00  2560 2608 2640 2720  1440 1443 1448 1481
>> +hsync -vsync
>>
>> With this mode and your new driver, I started getting rows of black
>> pixels
>> flickering in the upper-left corner of my LCD screen. These weren't
>> there back
>> when I was running the same mode under the Keith Zhao driver from
>> StarFive
>> (drm/verisilicon v5 plus a bunch of patches for dma cache coherency)
>> (
>> https://patchwork.kernel.org/project/dri-devel/cover/20241120061848.196754-1-keith.zhao@xxxxxxxxxxxxxxxx
>> /).
>>
>> The glitches I saw suggested the sync polarity was being set
>> incorrectly; the
>> number of glitched rows was exactly the vsync pulse width plus 1
>> (i.e.
>> "1440 1443 1448 1481" produced 6 bad rows, "1440 1443 1456 1481"
>> produced 14,
>> "1440 1441 1442 1481" produced 2, and so forth).
>>
>> I did a register dump between the two drivers and found that,
>> although the
>> DC8200 HSYNC and VSYNC registers were identical between the two
>> versions, the
>> hsync and vsync bits on the inno hdmi tx were swapped between the
>> two. When I
>> manually set the VIDEO_TIMING_CTL register from 0x09 to 0x05
>> (matching the
>> output of Keith's driver), the glitches went away.
>>
>> Keith Zhao driver:
>>
>> === DC8200 display 0 ===
>> DC   PANEL_CONFIG               0x1418 = 0x00001111
>> DC   HSIZE                      0x1430 = 0x0AA00A00
>> DC   HSYNC                      0x1438 = 0x45280A30
>> DC   VSIZE                      0x1440 = 0x05C905A0
>> DC   VSYNC                      0x1448 = 0xC2D405A3
>> DC   DPI_CONFIG                 0x14B8 = 0x00000005
>> DC   FB_CONFIG                  0x1518 = 0x14000000
>> DC   FB_ADDRESS                 0x1400 = 0x71200000
>> DC   FB_STRIDE                  0x1408 = 0x00002800
>> DC   FB_SIZE                    0x1810 = 0x02D00A00
>> DC   FB_CONFIG_EX               0x1CC0 = 0x00002000
>> DC   PANEL_START                0x1CCC = 0x00000001
>> DC   DP_CONFIG                  0x1CD0 = 0x0000000A
>> DC   FB_WATER_MARK              0x1CE8 = 0x00000000
>> DC   PANEL_CONFIG_EX            0x2518 = 0x00000000
>>
>> === INNO HDMI TX video timing ===
>> HDMI VIDEO_TIMING_CTL           0x08   = 0x05
>> HDMI EXT_HTOTAL_L               0x09   = 0xA0
>> HDMI EXT_HTOTAL_H               0x0A   = 0x0A
>> HDMI EXT_HBLANK_L               0x0B   = 0xA0
>> HDMI EXT_HBLANK_H               0x0C   = 0x00
>> HDMI EXT_HDELAY_L               0x0D   = 0x70
>> HDMI EXT_HDELAY_H               0x0E   = 0x00
>> HDMI EXT_HDURATION_L            0x0F   = 0x20
>> HDMI EXT_HDURATION_H            0x10   = 0x00
>> HDMI EXT_VTOTAL_L               0x11   = 0xC9
>> HDMI EXT_VTOTAL_H               0x12   = 0x05
>> HDMI EXT_VBLANK                 0x13   = 0x29
>> HDMI EXT_VDELAY                 0x14   = 0x26
>> HDMI EXT_VDURATION              0x15   = 0x05
>>
>> Your driver (showing only the changed values):
>>
>> DC   DP_CONFIG                  0x1CD0 = 0x00000000
>> DC   PANEL_CONFIG_EX            0x2518 = 0x00000001
>> HDMI VIDEO_TIMING_CTL           0x08   = 0x09
>>
>> Specifically the bit#s are different between the two drivers:
>>
>> inno_hdmi-starfive.h (Keith Zhao):     inno-hdmi.c (generic):
>>    v_HSYNC_POLARITY_SF(n)  ((n) << 2)     v_HSYNC_POLARITY  bit 3
>>    v_VSYNC_POLARITY_SF(n)  ((n) << 3)     v_VSYNC_POLARITY  bit 2
>>
>> "_SF" suggests this change is specific to StarFive. The correct
>> positions are
>> HSYNC=bit 2 and VSYNC=bit 3 for the JH7110.
> 
> In fact it's not SF specific, [1] pg.13 (full manual page 979) also
> shows the same for RK.
> 
> It could be an error from Rockchip developers since the beginning.

Yeah I agree; Maud Spierings reported similar issue and Dominuque
Belhachemi also figured that v_HSYNC_POLARITY and v_VSYNC_POLARITY are
simply swapped in
inno-hdmi.c.

As Icenowy stated Rockhip TRM also confirms this. I intend to send a fix
as part of this series (as suggested by Maud).

> 
> [1]
> https://protect2.fireeye.com/v1/url?k=ba283351-e5b30a4e-ba29b81e-000babdfecba-1b86e6ec0e74b45d&q=1&e=d9589fcd-304c-4065-91c9-c5043969df15&u=https%3A%2F%2Fgithub.com%2FDeciHD%2Frockchip_docs%2Fblob%2Fmain%2Frk3128%2FTRM-chapter-24-hdmi-tx.pdf
> 
> Thanks,
> Icenowy
> 
>>
>> I'm guessing that your 42-mode test passed because most CEA modes are
>> +hsync
>> +vsync, and this wouldn't show any difference if the two bits were
>> transposed.
>>
>> Once the sync polarity is sorted, I'd be happy to give a Tested-by:
>> on the
>> series.
>>
>>   --
>>
>> Issue 2: This is more a feature request than an issue. Originally, I
>> tried
>> running this Xorg modeline, which worked well under Keith's driver:
>>
>> "2560x1440_49.90"  201.00  2560 2608 2640 2720  1440 1443 1448 1481
>> +hsync -vsync
>>
>> However, your driver would not accept this pixel clock. I then
>> noticed you have
>> a pre-PLL table of fixed clocks. Is there any chance you can change
>> the code to
>> compute the pre-PLL on the fly like how Keith does it, rather than
>> use a table?

I can add the entry, hopefully you can let me know in the v5 whether it
works for you.

inno_hdmi_phy_pre_pll_determine_rate() returns -EINVAL for any rate not
in the table, and .mode_valid is built entirely on that:

rounded = clk_round_rate(ctrl->clks[CLK_PCLK].clk, pixelclk);
if (rounded != pixelclk)
      return MODE_NOCLOCK;

If the PHY computes dividers on demand, clk_round_rate() starts
succeeding for almost every rate .mode_valid stops rejecting anything
and we're back to advertising modes the PHY can't drive  the
blank screen bug patch 11 exists to fix.

To keep the rejection you'd have to replace the table with a real
constraint model.

The fracdiv values doesn't seem to be cleanly derivable and the Rockchip
has the datasheet and still chose a table. 


>>
>> Thanks for all your great work!
>>
>> Best regards,
>>   -Byron
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@xxxxxxxxxxxxxxxxxxx
>> https://protect2.fireeye.com/v1/url?k=46fbedc2-1960d4dd-46fa668d-000babdfecba-e6e7fb64f005c1b3&q=1&e=d9589fcd-304c-4065-91c9-c5043969df15&u=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-riscv

Best regards,
-- 
Michal Wilczynski <m.wilczynski@xxxxxxxxxxx>