Re: [PATCH RFC 0/2] Novatek NT51021 DSI panel IC driver

[Ryan Brue]

Hi Nickolay,

I sent a duplicate NT51021 driver on September 5th without finding
yours from the 29th first. That was my mistake, and I have withdrawn
mine. That's my bad, I'm new to kernel development :)

Since I had the hardware set up, I tested your series on the device I
am working on: the Amazon Fire HD 10 (2017), an MT8173 tablet with a BOE
1200x1920 NT51021 module, driven by mediatek-drm rather than msm. It
works.

Concretely, on that panel:

  - your 10.1" register sequence, timings and mode_flags, unmodified,
    bring the panel up correctly at your 60 Hz / 993.5 Mbps per lane
  - it also works with my vendor's own sequence and timings under your
    driver, at 62.5 Hz / 980 Mbps
  - verified across a cold boot, display off and on cycles and a
    suspend/resume, with the MIPI TX PLL reading back at the intended
    rate each time

So, for what it is worth on hardware you cannot test:

Tested-by: Ryan Brue <ryanbrue.dev@xxxxxxxxx>

Two things I checked that might save you answering them later:

  - The generic writes are right. Xiaomi's own panel dtsi uses dtype
    0x29, and this panel accepts both generic and DCS short writes, so
    nothing depends on the choice.

  - prepare() never sending a set_display_on looked like an omission to
    me at first, but the 0x01 software reset at the head of the init
    restores it, and repeated off and on cycles pass. It may be worth a
    comment, since unprepare() does send set_display_off and the
    asymmetry invites the question.

One question on the binding, which is as much about my patch as yours.
Where does "boe,tv101wum-nx0" come from? Xiaomi's panel dtsi
(dsi-panel-nt51021-boe-10wu-video.dtsi) gives only

    qcom,mdss-dsi-panel-name = "NT51021_BOE_BOE10";

with no part number, and the register sequence in your driver matches
that file byte for byte. TV101WUM-NX0 is a real BOE part, and its
datasheet active area of 216.576 x 135.36 mm agrees nicely with the
135 x 216 in that dtsi, so the attribution looks reasonable. But if it
is inferred rather than read off the panel or a BOM, it may be worth
saying so in the commit message, or naming the compatible after the
board the way panel-novatek-nt36523.c does for
xiaomi,elish-boe-nt36523.

I ask because my module has no identity I can establish at all. It
answers no DSI read, DCS or generic, before or after sleep-out; the
vendor kernel and bootloader name it only "BOE", chosen from two board
ID straps; and its vendor-declared physical height is arithmetically
impossible, since 1200/136 and 1920/221 disagree by 1.5%. So I am
naming mine after the board, and would rather not accidentally imply it
is or is not your part.

If it is useful, I have the Fire HD 10 panel ready as a variant on top
of your series: two patches, binding and driver, adding
amazon,suez-boe-nt51021. The only change it needs in your code is
making mode_flags per variant. My vendor asks for sync-event video mode
with a continuously running HS clock rather than burst, and your two
panels keep exactly the flags they have now. Burst worked on my panel
too, so this is about matching what the module is characterised for,
not a fix.

I will hold that until your v2, since Neil's renaming will touch every
identifier it adds. Happy to test v2 on this hardware when you post it.

Thanks for doing this first.

Ryan