diff mbox series

[v2,2/4] drm/panel: Add driver for DJN HX83112A LCD panel

Message ID 20240110-fp4-panel-v2-2-8ad11174f65b@fairphone.com (mailing list archive)
State New, archived
Headers show
Series Add display support for Fairphone 4 | expand

Commit Message

Luca Weiss Jan. 10, 2024, 3:14 p.m. UTC
Add support for the 2340x1080 LCD DJN panel bundled with a HX83112A
driver IC, as found on the Fairphone 4 smartphone.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 drivers/gpu/drm/panel/Kconfig                |  10 +
 drivers/gpu/drm/panel/Makefile               |   1 +
 drivers/gpu/drm/panel/panel-himax-hx83112a.c | 352 +++++++++++++++++++++++++++
 3 files changed, 363 insertions(+)

Comments

Linus Walleij Jan. 11, 2024, 2:57 p.m. UTC | #1
On Wed, Jan 10, 2024 at 4:14 PM Luca Weiss <luca.weiss@fairphone.com> wrote:

> Add support for the 2340x1080 LCD DJN panel bundled with a HX83112A
> driver IC, as found on the Fairphone 4 smartphone.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>

This makes it sound like the Himax HX83112A is the driver IC.

This does not seem to be the case:
https://github.com/HimaxSoftware/HX83112_Android_Driver

The driver IC seems to be a truly 5p65.

If this is right, could you rename the driver file to truly-5p65.c
and all symbols containing hx83112 to truly_5965 or something
that indicate the driver IC instead of the panel?

My main concern is that the next display using the same IC
need to find the right file to patch.

Yours,
Linus Walleij
Luca Weiss Jan. 11, 2024, 3:28 p.m. UTC | #2
On Thu Jan 11, 2024 at 3:57 PM CET, Linus Walleij wrote:
> On Wed, Jan 10, 2024 at 4:14 PM Luca Weiss <luca.weiss@fairphone.com> wrote:
>
> > Add support for the 2340x1080 LCD DJN panel bundled with a HX83112A
> > driver IC, as found on the Fairphone 4 smartphone.
> >
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>
> This makes it sound like the Himax HX83112A is the driver IC.
>
> This does not seem to be the case:
> https://github.com/HimaxSoftware/HX83112_Android_Driver
>
> The driver IC seems to be a truly 5p65.

Hi Linus,

In some internal documentation it says "LCD Driver IC" "HX83112A" and I
don't see any reference to Truly 5P65 anywhere.

On their website they have this sentence:

  Himax offers display drivers for mobile handset displays that combine
  source driver, gate driver, timing controller, frame buffer, and DC to
  DC circuits into a single chip in various display technologies, such
  as TFT-LCD, LTPS, In-Cell Touch and AMOLED.

https://www.himax.com.tw/products/display-drivers/mobile-handset-applications/

While I'm not super well versed in panel driver ICs, this sounds like it
should be the one to take the name from?

Regards
Luca

>
> If this is right, could you rename the driver file to truly-5p65.c
> and all symbols containing hx83112 to truly_5965 or something
> that indicate the driver IC instead of the panel?
>
> My main concern is that the next display using the same IC
> need to find the right file to patch.
>
> Yours,
> Linus Walleij
Linus Walleij Jan. 11, 2024, 7:05 p.m. UTC | #3
On Thu, Jan 11, 2024 at 4:28 PM Luca Weiss <luca.weiss@fairphone.com> wrote:

> In some internal documentation it says "LCD Driver IC" "HX83112A" and I
> don't see any reference to Truly 5P65 anywhere.

In the Android directory I pointed to I see this file:
HX83112_Android_Driver/Truly_5p65_module_fw/UpdateFW.bat

(Notice the 5p65 fw dir is *inside* the HX82112 dir)

And in that file:
adb push TRULY_5P65_1080_2160_HX83112A_D01C01.bin
/system/etc/firmware/Himax_firmware.bin

Clearly indicating that they are pushing a Truly 5P65 firmware into
the Himax display firmware directory.

To be fair, that is the driver for the touchscreen part of HX83112A,
but ... Truly is a well known manufacturer of display controllers?

But... given that you have a @fairphone.com mal address and
a working relationship with them, can't you just ask?

> On their website they have this sentence:

All OEMs want to look like everything is their own product. It is
business as usual.

Further on the same note since I guess you have a datasheet)
please bring in #defines for the commands (the first byte in the
write sequences, for examples:

+       mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x02);
+       mipi_dsi_dcs_write_seq(dsi, 0xd8,
+                              0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
+                              0xff, 0xff, 0xff, 0xff);
+       mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x03);

Clearly 0xbd is HX83112_CMD_SETREGBANK?
(This is easily spotted from the similar structure in the
existing panel-himax-hx8394.c.) So please add #defines
for all commands you know, especially if you have a datasheet
because we reviewers don't have them and "it's just magic
bytes" isn't very compelling. It adds a lot to understanding.

I strongly suspect other Himax displays such as HX8394 to
be using a Truly controller as well, hence the similarities.

In a datasheet for their TFT800480-84-V1-E display controller
Truly kept the init sequence name of void LCD_INIT_HX8290(void)
for example.

Yours,
Linus Walleij
Luca Weiss Jan. 12, 2024, 9 a.m. UTC | #4
On Thu Jan 11, 2024 at 8:05 PM CET, Linus Walleij wrote:
> On Thu, Jan 11, 2024 at 4:28 PM Luca Weiss <luca.weiss@fairphone.com> wrote:
>
> > In some internal documentation it says "LCD Driver IC" "HX83112A" and I
> > don't see any reference to Truly 5P65 anywhere.
>
> In the Android directory I pointed to I see this file:
> HX83112_Android_Driver/Truly_5p65_module_fw/UpdateFW.bat
>
> (Notice the 5p65 fw dir is *inside* the HX82112 dir)
>
> And in that file:
> adb push TRULY_5P65_1080_2160_HX83112A_D01C01.bin
> /system/etc/firmware/Himax_firmware.bin
>
> Clearly indicating that they are pushing a Truly 5P65 firmware into
> the Himax display firmware directory.
>
> To be fair, that is the driver for the touchscreen part of HX83112A,
> but ... Truly is a well known manufacturer of display controllers?
>
> But... given that you have a @fairphone.com mal address and
> a working relationship with them, can't you just ask?
>
> > On their website they have this sentence:
>
> All OEMs want to look like everything is their own product. It is
> business as usual.

I can't tell you anything there that I don't know, sorry.

>
> Further on the same note since I guess you have a datasheet)
> please bring in #defines for the commands (the first byte in the
> write sequences, for examples:
>
> +       mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x02);
> +       mipi_dsi_dcs_write_seq(dsi, 0xd8,
> +                              0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
> +                              0xff, 0xff, 0xff, 0xff);
> +       mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x03);
>
> Clearly 0xbd is HX83112_CMD_SETREGBANK?
> (This is easily spotted from the similar structure in the
> existing panel-himax-hx8394.c.) So please add #defines
> for all commands you know, especially if you have a datasheet
> because we reviewers don't have them and "it's just magic
> bytes" isn't very compelling. It adds a lot to understanding.

Right, the register names seem to match more or less the ones from that
driver, plus some new ones and some differently named ones. Will send a
v3 with that.

>
> I strongly suspect other Himax displays such as HX8394 to
> be using a Truly controller as well, hence the similarities.
>
> In a datasheet for their TFT800480-84-V1-E display controller
> Truly kept the init sequence name of void LCD_INIT_HX8290(void)
> for example.

In that datasheet (assuming I'm looking at the same one?) it says
"Driver IC" "HX8290-A[...]" so there the display driver is manufactured
by Himax and not Truly to my understanding. Truly is assembling together
Driver + all the other parts that go into an LCD.

For the panel used on Fairphone 4 that part is done by the company DJN.

Regards
Luca

>
> Yours,
> Linus Walleij
Neil Armstrong Jan. 12, 2024, 9:14 a.m. UTC | #5
On 12/01/2024 10:00, Luca Weiss wrote:
> On Thu Jan 11, 2024 at 8:05 PM CET, Linus Walleij wrote:
>> On Thu, Jan 11, 2024 at 4:28 PM Luca Weiss <luca.weiss@fairphone.com> wrote:
>>
>>> In some internal documentation it says "LCD Driver IC" "HX83112A" and I
>>> don't see any reference to Truly 5P65 anywhere.
>>
>> In the Android directory I pointed to I see this file:
>> HX83112_Android_Driver/Truly_5p65_module_fw/UpdateFW.bat
>>
>> (Notice the 5p65 fw dir is *inside* the HX82112 dir)
>>
>> And in that file:
>> adb push TRULY_5P65_1080_2160_HX83112A_D01C01.bin
>> /system/etc/firmware/Himax_firmware.bin
>>
>> Clearly indicating that they are pushing a Truly 5P65 firmware into
>> the Himax display firmware directory.
>>
>> To be fair, that is the driver for the touchscreen part of HX83112A,
>> but ... Truly is a well known manufacturer of display controllers?
>>
>> But... given that you have a @fairphone.com mal address and
>> a working relationship with them, can't you just ask?
>>
>>> On their website they have this sentence:
>>
>> All OEMs want to look like everything is their own product. It is
>> business as usual.
> 
> I can't tell you anything there that I don't know, sorry.
> 
>>
>> Further on the same note since I guess you have a datasheet)
>> please bring in #defines for the commands (the first byte in the
>> write sequences, for examples:
>>
>> +       mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x02);
>> +       mipi_dsi_dcs_write_seq(dsi, 0xd8,
>> +                              0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
>> +                              0xff, 0xff, 0xff, 0xff);
>> +       mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x03);
>>
>> Clearly 0xbd is HX83112_CMD_SETREGBANK?
>> (This is easily spotted from the similar structure in the
>> existing panel-himax-hx8394.c.) So please add #defines
>> for all commands you know, especially if you have a datasheet
>> because we reviewers don't have them and "it's just magic
>> bytes" isn't very compelling. It adds a lot to understanding.
> 
> Right, the register names seem to match more or less the ones from that
> driver, plus some new ones and some differently named ones. Will send a
> v3 with that.
> 
>>
>> I strongly suspect other Himax displays such as HX8394 to
>> be using a Truly controller as well, hence the similarities.
>>
>> In a datasheet for their TFT800480-84-V1-E display controller
>> Truly kept the init sequence name of void LCD_INIT_HX8290(void)
>> for example.
> 
> In that datasheet (assuming I'm looking at the same one?) it says
> "Driver IC" "HX8290-A[...]" so there the display driver is manufactured
> by Himax and not Truly to my understanding. Truly is assembling together
> Driver + all the other parts that go into an LCD.
> 
> For the panel used on Fairphone 4 that part is done by the company DJN.

Looking at the discussion, this seems to confirm the Display+Touch IC is HX83112A,
and Truly is the panel manufacturer and all assembled by DJN, so IMHO the initial driver is right.

Perhaps the compatible should be djn,hx83112a-truly-5p65 to reflect that ?

Neil

> 
> Regards
> Luca
> 
>>
>> Yours,
>> Linus Walleij
>
Luca Weiss Jan. 12, 2024, 9:51 a.m. UTC | #6
On Fri Jan 12, 2024 at 10:14 AM CET, Neil Armstrong wrote:
> On 12/01/2024 10:00, Luca Weiss wrote:
> > On Thu Jan 11, 2024 at 8:05 PM CET, Linus Walleij wrote:
> >> On Thu, Jan 11, 2024 at 4:28 PM Luca Weiss <luca.weiss@fairphone.com> wrote:
> >>
> >>> In some internal documentation it says "LCD Driver IC" "HX83112A" and I
> >>> don't see any reference to Truly 5P65 anywhere.
> >>
> >> In the Android directory I pointed to I see this file:
> >> HX83112_Android_Driver/Truly_5p65_module_fw/UpdateFW.bat
> >>
> >> (Notice the 5p65 fw dir is *inside* the HX82112 dir)
> >>
> >> And in that file:
> >> adb push TRULY_5P65_1080_2160_HX83112A_D01C01.bin
> >> /system/etc/firmware/Himax_firmware.bin
> >>
> >> Clearly indicating that they are pushing a Truly 5P65 firmware into
> >> the Himax display firmware directory.
> >>
> >> To be fair, that is the driver for the touchscreen part of HX83112A,
> >> but ... Truly is a well known manufacturer of display controllers?
> >>
> >> But... given that you have a @fairphone.com mal address and
> >> a working relationship with them, can't you just ask?
> >>
> >>> On their website they have this sentence:
> >>
> >> All OEMs want to look like everything is their own product. It is
> >> business as usual.
> > 
> > I can't tell you anything there that I don't know, sorry.
> > 
> >>
> >> Further on the same note since I guess you have a datasheet)
> >> please bring in #defines for the commands (the first byte in the
> >> write sequences, for examples:
> >>
> >> +       mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x02);
> >> +       mipi_dsi_dcs_write_seq(dsi, 0xd8,
> >> +                              0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
> >> +                              0xff, 0xff, 0xff, 0xff);
> >> +       mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x03);
> >>
> >> Clearly 0xbd is HX83112_CMD_SETREGBANK?
> >> (This is easily spotted from the similar structure in the
> >> existing panel-himax-hx8394.c.) So please add #defines
> >> for all commands you know, especially if you have a datasheet
> >> because we reviewers don't have them and "it's just magic
> >> bytes" isn't very compelling. It adds a lot to understanding.
> > 
> > Right, the register names seem to match more or less the ones from that
> > driver, plus some new ones and some differently named ones. Will send a
> > v3 with that.
> > 
> >>
> >> I strongly suspect other Himax displays such as HX8394 to
> >> be using a Truly controller as well, hence the similarities.
> >>
> >> In a datasheet for their TFT800480-84-V1-E display controller
> >> Truly kept the init sequence name of void LCD_INIT_HX8290(void)
> >> for example.
> > 
> > In that datasheet (assuming I'm looking at the same one?) it says
> > "Driver IC" "HX8290-A[...]" so there the display driver is manufactured
> > by Himax and not Truly to my understanding. Truly is assembling together
> > Driver + all the other parts that go into an LCD.
> > 
> > For the panel used on Fairphone 4 that part is done by the company DJN.
>
> Looking at the discussion, this seems to confirm the Display+Touch IC is HX83112A,
> and Truly is the panel manufacturer and all assembled by DJN, so IMHO the initial driver is right.
>
> Perhaps the compatible should be djn,hx83112a-truly-5p65 to reflect that ?

Since there's zero indication Truly is involved in this panel in my
documentation - much less the number 5P65 - I'm not going to add that.

One other number I'm certain of is from DJN's side the model number of
this panel is 9A-3R063-1102B, which I assume is the specific combination
of components + frame and everything for Fairphone 4 device.

That one you can also find in this document (Ctrl-F for DJN)
https://www.fairphone.com/wp-content/uploads/2022/09/FP4_Information-for-repairers-and-recyclers.pdf
.. or on this picture:
https://guide-images.cdn.ifixit.com/igi/HgTquQPABg1mAMHD.huge

So something like djn,9a-3r063-1102b would also be somewhat valid I
guess?

So in short this panel is the model 9A-3R063-1102B from DJN, which uses
a Himax HX83112A driver IC.

And there's also AU Optronics listed as 玻璃厂家 ("glass manufacturer"?)
fwiw, though the display also uses Corning Gorilla Glass 5 so not sure
who's supplying what.

Regards
Luca

>
> Neil
>
> > 
> > Regards
> > Luca
> > 
> >>
> >> Yours,
> >> Linus Walleij
> >
Linus Walleij Jan. 12, 2024, 10:23 a.m. UTC | #7
On Fri, Jan 12, 2024 at 10:52 AM Luca Weiss <luca.weiss@fairphone.com> wrote:

> Since there's zero indication Truly is involved in this panel in my
> documentation - much less the number 5P65 - I'm not going to add that.

OK then, I fold, thanks for looking into it.
Keep the Himax hx83112a file name and symbols.

> So in short this panel is the model 9A-3R063-1102B from DJN, which uses
> a Himax HX83112A driver IC.

So compatible = "djn,9a-3r063-1102b" since the setup sequences for
hx83112a are clearly for this one display?

Yours,
Linus Walleij
Neil Armstrong Jan. 12, 2024, 10:26 a.m. UTC | #8
On 12/01/2024 11:23, Linus Walleij wrote:
> On Fri, Jan 12, 2024 at 10:52 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
> 
>> Since there's zero indication Truly is involved in this panel in my
>> documentation - much less the number 5P65 - I'm not going to add that.

Ack

> 
> OK then, I fold, thanks for looking into it.
> Keep the Himax hx83112a file name and symbols.
> 
>> So in short this panel is the model 9A-3R063-1102B from DJN, which uses
>> a Himax HX83112A driver IC.
> 
> So compatible = "djn,9a-3r063-1102b" since the setup sequences for
> hx83112a are clearly for this one display?

Yep let's settle on that!

Thanks,
Neil

> 
> Yours,
> Linus Walleij
Luca Weiss Jan. 22, 2024, 11:27 a.m. UTC | #9
On Fri Jan 12, 2024 at 11:26 AM CET,  wrote:
> On 12/01/2024 11:23, Linus Walleij wrote:
> > On Fri, Jan 12, 2024 at 10:52 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
> > 
> >> Since there's zero indication Truly is involved in this panel in my
> >> documentation - much less the number 5P65 - I'm not going to add that.
>
> Ack
>
> > 
> > OK then, I fold, thanks for looking into it.
> > Keep the Himax hx83112a file name and symbols.
> > 
> >> So in short this panel is the model 9A-3R063-1102B from DJN, which uses
> >> a Himax HX83112A driver IC.
> > 
> > So compatible = "djn,9a-3r063-1102b" since the setup sequences for
> > hx83112a are clearly for this one display?
>
> Yep let's settle on that!

It's clear to me to use "djn,9a-3r063-1102b" in the driver now but what
about dts?

Currently here in v2 we have this:
compatible = "fairphone,fp4-hx83112a-djn", "himax,hx83112a";

Should this just become this?
compatible = "djn,9a-3r063-1102b";

Or e.g. this?
compatible = "djn,9a-3r063-1102b", "himax,hx83112a";

Or something else completely? Do we have some documentation / best
practises around this maybe?

Regards
Luca

>
> Thanks,
> Neil
>
> > 
> > Yours,
> > Linus Walleij
Luca Weiss Feb. 14, 2024, 9:33 a.m. UTC | #10
On Mon Jan 22, 2024 at 12:27 PM CET, Luca Weiss wrote:
> On Fri Jan 12, 2024 at 11:26 AM CET,  wrote:
> > On 12/01/2024 11:23, Linus Walleij wrote:
> > > On Fri, Jan 12, 2024 at 10:52 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
> > > 
> > >> Since there's zero indication Truly is involved in this panel in my
> > >> documentation - much less the number 5P65 - I'm not going to add that.
> >
> > Ack
> >
> > > 
> > > OK then, I fold, thanks for looking into it.
> > > Keep the Himax hx83112a file name and symbols.
> > > 
> > >> So in short this panel is the model 9A-3R063-1102B from DJN, which uses
> > >> a Himax HX83112A driver IC.
> > > 
> > > So compatible = "djn,9a-3r063-1102b" since the setup sequences for
> > > hx83112a are clearly for this one display?
> >
> > Yep let's settle on that!
>

Hi Neil and Linus,

Any feedback about the below question?

Regards
Luca

> It's clear to me to use "djn,9a-3r063-1102b" in the driver now but what
> about dts?
>
> Currently here in v2 we have this:
> compatible = "fairphone,fp4-hx83112a-djn", "himax,hx83112a";
>
> Should this just become this?
> compatible = "djn,9a-3r063-1102b";
>
> Or e.g. this?
> compatible = "djn,9a-3r063-1102b", "himax,hx83112a";
>
> Or something else completely? Do we have some documentation / best
> practises around this maybe?
>
> Regards
> Luca
>
> >
> > Thanks,
> > Neil
> >
> > > 
> > > Yours,
> > > Linus Walleij
Neil Armstrong Feb. 14, 2024, 9:50 a.m. UTC | #11
On 14/02/2024 10:33, Luca Weiss wrote:
> On Mon Jan 22, 2024 at 12:27 PM CET, Luca Weiss wrote:
>> On Fri Jan 12, 2024 at 11:26 AM CET,  wrote:
>>> On 12/01/2024 11:23, Linus Walleij wrote:
>>>> On Fri, Jan 12, 2024 at 10:52 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
>>>>
>>>>> Since there's zero indication Truly is involved in this panel in my
>>>>> documentation - much less the number 5P65 - I'm not going to add that.
>>>
>>> Ack
>>>
>>>>
>>>> OK then, I fold, thanks for looking into it.
>>>> Keep the Himax hx83112a file name and symbols.
>>>>
>>>>> So in short this panel is the model 9A-3R063-1102B from DJN, which uses
>>>>> a Himax HX83112A driver IC.
>>>>
>>>> So compatible = "djn,9a-3r063-1102b" since the setup sequences for
>>>> hx83112a are clearly for this one display?
>>>
>>> Yep let's settle on that!
>>
> 
> Hi Neil and Linus,
> 
> Any feedback about the below question?
> 
> Regards
> Luca
> 
>> It's clear to me to use "djn,9a-3r063-1102b" in the driver now but what
>> about dts?
>>
>> Currently here in v2 we have this:
>> compatible = "fairphone,fp4-hx83112a-djn", "himax,hx83112a";
>>
>> Should this just become this?
>> compatible = "djn,9a-3r063-1102b";
>>
>> Or e.g. this?
>> compatible = "djn,9a-3r063-1102b", "himax,hx83112a";
>>
>> Or something else completely? Do we have some documentation / best
>> practises around this maybe?

Sorry I totally missed the question.

Not sure if "himax,hx83112a" is needed here, the "djn,9a-3r063-1102b" is enough to know the IC is hx83112a.

I don't think you'll ever find a "djn,9a-3r063-1102b" with another controller IC ?

And "himax,hx83112a" alone as fallback is not enough to describe the panel hardware, so I think it should be dropped.

Thanks,
Neil

>>
>> Regards
>> Luca
>>
>>>
>>> Thanks,
>>> Neil
>>>
>>>>
>>>> Yours,
>>>> Linus Walleij
>
Konrad Dybcio Feb. 15, 2024, 10:41 a.m. UTC | #12
On 14.02.2024 10:50, neil.armstrong@linaro.org wrote:
> On 14/02/2024 10:33, Luca Weiss wrote:
>> On Mon Jan 22, 2024 at 12:27 PM CET, Luca Weiss wrote:
>>> On Fri Jan 12, 2024 at 11:26 AM CET,  wrote:
>>>> On 12/01/2024 11:23, Linus Walleij wrote:
>>>>> On Fri, Jan 12, 2024 at 10:52 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
>>>>>
>>>>>> Since there's zero indication Truly is involved in this panel in my
>>>>>> documentation - much less the number 5P65 - I'm not going to add that.
>>>>
>>>> Ack
>>>>
>>>>>
>>>>> OK then, I fold, thanks for looking into it.
>>>>> Keep the Himax hx83112a file name and symbols.
>>>>>
>>>>>> So in short this panel is the model 9A-3R063-1102B from DJN, which uses
>>>>>> a Himax HX83112A driver IC.
>>>>>
>>>>> So compatible = "djn,9a-3r063-1102b" since the setup sequences for
>>>>> hx83112a are clearly for this one display?
>>>>
>>>> Yep let's settle on that!
>>>
>>
>> Hi Neil and Linus,
>>
>> Any feedback about the below question?
>>
>> Regards
>> Luca
>>
>>> It's clear to me to use "djn,9a-3r063-1102b" in the driver now but what
>>> about dts?
>>>
>>> Currently here in v2 we have this:
>>> compatible = "fairphone,fp4-hx83112a-djn", "himax,hx83112a";
>>>
>>> Should this just become this?
>>> compatible = "djn,9a-3r063-1102b";
>>>
>>> Or e.g. this?
>>> compatible = "djn,9a-3r063-1102b", "himax,hx83112a";
>>>
>>> Or something else completely? Do we have some documentation / best
>>> practises around this maybe?
> 
> Sorry I totally missed the question.
> 
> Not sure if "himax,hx83112a" is needed here, the "djn,9a-3r063-1102b" is enough to know the IC is hx83112a.
> 
> I don't think you'll ever find a "djn,9a-3r063-1102b" with another controller IC ?
> 
> And "himax,hx83112a" alone as fallback is not enough to describe the panel hardware, so I think it should be dropped.

+1

Konrad
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 99e14dc212ec..3379b13df4b8 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -162,6 +162,16 @@  config DRM_PANEL_FEIYANG_FY07024DI26A30D
 	  Say Y if you want to enable support for panels based on the
 	  Feiyang FY07024DI26A30-D MIPI-DSI interface.
 
+config DRM_PANEL_HIMAX_HX83112A
+	tristate "Himax HX83112A-based DSI panel"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	select DRM_KMS_HELPER
+	help
+	  Say Y here if you want to enable support for Himax HX83112A-based
+	  display panels, such as the one found in the Fairphone 4 smartphone.
+
 config DRM_PANEL_HIMAX_HX8394
 	tristate "HIMAX HX8394 MIPI-DSI LCD panels"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index d10c3de51c6d..c2fc4c8c8340 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -14,6 +14,7 @@  obj-$(CONFIG_DRM_PANEL_EBBG_FT8719) += panel-ebbg-ft8719.o
 obj-$(CONFIG_DRM_PANEL_ELIDA_KD35T133) += panel-elida-kd35t133.o
 obj-$(CONFIG_DRM_PANEL_FEIXIN_K101_IM2BA02) += panel-feixin-k101-im2ba02.o
 obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d.o
+obj-$(CONFIG_DRM_PANEL_HIMAX_HX83112A) += panel-himax-hx83112a.o
 obj-$(CONFIG_DRM_PANEL_HIMAX_HX8394) += panel-himax-hx8394.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += panel-ilitek-ili9341.o
diff --git a/drivers/gpu/drm/panel/panel-himax-hx83112a.c b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
new file mode 100644
index 000000000000..cccc6075fd8e
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
@@ -0,0 +1,352 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generated with linux-mdss-dsi-panel-driver-generator from vendor device tree.
+ * Copyright (c) 2024 Luca Weiss <luca.weiss@fairphone.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regulator/consumer.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_probe_helper.h>
+
+struct hx83112a_panel {
+	struct drm_panel panel;
+	struct mipi_dsi_device *dsi;
+	struct regulator_bulk_data supplies[3];
+	struct gpio_desc *reset_gpio;
+};
+
+static inline struct hx83112a_panel *to_hx83112a_panel(struct drm_panel *panel)
+{
+	return container_of(panel, struct hx83112a_panel, panel);
+}
+
+static void hx83112a_reset(struct hx83112a_panel *ctx)
+{
+	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+	msleep(20);
+	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+	msleep(20);
+	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+	msleep(50);
+}
+
+static int hx83112a_on(struct hx83112a_panel *ctx)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	mipi_dsi_dcs_write_seq(dsi, 0xb9, 0x83, 0x11, 0x2a);
+	mipi_dsi_dcs_write_seq(dsi, 0xb1,
+			       0x08, 0x28, 0x28, 0x83, 0x83, 0x4c, 0x4f, 0x33);
+	mipi_dsi_dcs_write_seq(dsi, 0xb2,
+			       0x00, 0x02, 0x00, 0x90, 0x24, 0x00, 0x08, 0x19,
+			       0xea, 0x11, 0x11, 0x00, 0x11, 0xa3);
+	mipi_dsi_dcs_write_seq(dsi, 0xb4,
+			       0x58, 0x68, 0x58, 0x68, 0x0f, 0xef, 0x0b, 0xc0,
+			       0x0b, 0xc0, 0x0b, 0xc0, 0x00, 0xff, 0x00, 0xff,
+			       0x00, 0x00, 0x14, 0x15, 0x00, 0x29, 0x11, 0x07,
+			       0x12, 0x00, 0x29);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x02);
+	mipi_dsi_dcs_write_seq(dsi, 0xb4,
+			       0x00, 0x12, 0x12, 0x11, 0x88, 0x12, 0x12, 0x00,
+			       0x53);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x00);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x03);
+	mipi_dsi_dcs_write_seq(dsi, 0xc1,
+			       0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
+			       0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
+			       0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
+			       0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
+			       0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
+			       0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
+			       0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
+			       0x40);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x02);
+	mipi_dsi_dcs_write_seq(dsi, 0xc1,
+			       0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
+			       0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
+			       0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
+			       0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
+			       0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
+			       0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
+			       0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
+			       0x40);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x01);
+	mipi_dsi_dcs_write_seq(dsi, 0xc1,
+			       0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
+			       0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
+			       0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
+			       0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
+			       0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
+			       0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
+			       0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
+			       0x40);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x00);
+	mipi_dsi_dcs_write_seq(dsi, 0xc1, 0x01);
+	mipi_dsi_dcs_write_seq(dsi, 0xc7, 0x70, 0x00, 0x04, 0xe0, 0x33, 0x00);
+	mipi_dsi_dcs_write_seq(dsi, 0xcc, 0x08);
+	mipi_dsi_dcs_write_seq(dsi, 0xd2, 0x2b, 0x2b);
+	mipi_dsi_dcs_write_seq(dsi, 0xd3,
+			       0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x08,
+			       0x08, 0x03, 0x03, 0x22, 0x18, 0x07, 0x07, 0x07,
+			       0x07, 0x32, 0x10, 0x06, 0x00, 0x06, 0x32, 0x10,
+			       0x07, 0x00, 0x07, 0x32, 0x19, 0x31, 0x09, 0x31,
+			       0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x08,
+			       0x09, 0x30, 0x00, 0x00, 0x00, 0x06, 0x0d, 0x00,
+			       0x0f);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x01);
+	mipi_dsi_dcs_write_seq(dsi, 0xd3,
+			       0x00, 0x00, 0x19, 0x10, 0x00, 0x0a, 0x00, 0x81);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x00);
+	mipi_dsi_dcs_write_seq(dsi, 0xd5,
+			       0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
+			       0xc0, 0xc0, 0x18, 0x18, 0x19, 0x19, 0x18, 0x18,
+			       0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
+			       0x28, 0x28, 0x24, 0x24, 0x02, 0x03, 0x02, 0x03,
+			       0x00, 0x01, 0x00, 0x01, 0x31, 0x31, 0x31, 0x31,
+			       0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
+	mipi_dsi_dcs_write_seq(dsi, 0xd6,
+			       0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
+			       0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x19, 0x19,
+			       0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
+			       0x24, 0x24, 0x28, 0x28, 0x01, 0x00, 0x01, 0x00,
+			       0x03, 0x02, 0x03, 0x02, 0x31, 0x31, 0x31, 0x31,
+			       0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
+	mipi_dsi_dcs_write_seq(dsi, 0xd8,
+			       0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea,
+			       0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa,
+			       0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa, 0xaa, 0xaa);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x01);
+	mipi_dsi_dcs_write_seq(dsi, 0xd8,
+			       0xaa, 0x2e, 0x28, 0x00, 0x00, 0x00, 0xaa, 0x2e,
+			       0x28, 0x00, 0x00, 0x00, 0xaa, 0xee, 0xaa, 0xaa,
+			       0xaa, 0xaa, 0xaa, 0xee, 0xaa, 0xaa, 0xaa, 0xaa);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x02);
+	mipi_dsi_dcs_write_seq(dsi, 0xd8,
+			       0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
+			       0xff, 0xff, 0xff, 0xff);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x03);
+	mipi_dsi_dcs_write_seq(dsi, 0xd8,
+			       0xaa, 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
+			       0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xff, 0xff, 0xff,
+			       0xff, 0xff, 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x00);
+	mipi_dsi_dcs_write_seq(dsi, 0xe7,
+			       0x0e, 0x0e, 0x1e, 0x65, 0x1c, 0x65, 0x00, 0x50,
+			       0x20, 0x20, 0x00, 0x00, 0x02, 0x02, 0x02, 0x05,
+			       0x14, 0x14, 0x32, 0xb9, 0x23, 0xb9, 0x08);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x01);
+	mipi_dsi_dcs_write_seq(dsi, 0xe7,
+			       0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x02);
+	mipi_dsi_dcs_write_seq(dsi, 0xe7,
+			       0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x00, 0x00,
+			       0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+			       0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
+			       0x00, 0x00, 0x00, 0x02, 0x00);
+	mipi_dsi_dcs_write_seq(dsi, 0xbd, 0x00);
+	mipi_dsi_dcs_write_seq(dsi, 0xe9, 0xc3);
+	mipi_dsi_dcs_write_seq(dsi, 0xcb, 0xd1, 0xd6);
+	mipi_dsi_dcs_write_seq(dsi, 0xe9, 0x3f);
+	mipi_dsi_dcs_write_seq(dsi, 0xe9, 0xc6);
+	mipi_dsi_dcs_write_seq(dsi, 0xbf, 0x37);
+	mipi_dsi_dcs_write_seq(dsi, 0xe9, 0x3f);
+
+	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
+		return ret;
+	}
+	msleep(150);
+
+	ret = mipi_dsi_dcs_set_display_on(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set display on: %d\n", ret);
+		return ret;
+	}
+	msleep(50);
+
+	return 0;
+}
+
+static int hx83112a_disable(struct drm_panel *panel)
+{
+	struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	ret = mipi_dsi_dcs_set_display_off(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set display off: %d\n", ret);
+		return ret;
+	}
+	msleep(20);
+
+	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
+		return ret;
+	}
+	msleep(120);
+
+	return 0;
+}
+
+static int hx83112a_prepare(struct drm_panel *panel)
+{
+	struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
+	struct device *dev = &ctx->dsi->dev;
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
+	hx83112a_reset(ctx);
+
+	ret = hx83112a_on(ctx);
+	if (ret < 0) {
+		dev_err(dev, "Failed to initialize panel: %d\n", ret);
+		gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+		regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int hx83112a_unprepare(struct drm_panel *panel)
+{
+	struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
+
+	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+	regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+
+	return 0;
+}
+
+static const struct drm_display_mode hx83112a_mode = {
+	.clock = (1080 + 28 + 8 + 8) * (2340 + 27 + 5 + 5) * 60 / 1000,
+	.hdisplay = 1080,
+	.hsync_start = 1080 + 28,
+	.hsync_end = 1080 + 28 + 8,
+	.htotal = 1080 + 28 + 8 + 8,
+	.vdisplay = 2340,
+	.vsync_start = 2340 + 27,
+	.vsync_end = 2340 + 27 + 5,
+	.vtotal = 2340 + 27 + 5 + 5,
+	.width_mm = 67,
+	.height_mm = 145,
+	.type = DRM_MODE_TYPE_DRIVER,
+};
+
+static int hx83112a_get_modes(struct drm_panel *panel,
+				  struct drm_connector *connector)
+{
+	return drm_connector_helper_get_modes_fixed(connector, &hx83112a_mode);
+}
+
+static const struct drm_panel_funcs hx83112a_panel_funcs = {
+	.prepare = hx83112a_prepare,
+	.unprepare = hx83112a_unprepare,
+	.disable = hx83112a_disable,
+	.get_modes = hx83112a_get_modes,
+};
+
+static int hx83112a_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct hx83112a_panel *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->supplies[0].supply = "vdd1";
+	ctx->supplies[1].supply = "vsn";
+	ctx->supplies[2].supply = "vsp";
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
+				      ctx->supplies);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to get regulators\n");
+
+	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ctx->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
+				     "Failed to get reset-gpios\n");
+
+	ctx->dsi = dsi;
+	mipi_dsi_set_drvdata(dsi, ctx);
+
+	dsi->lanes = 4;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+			  MIPI_DSI_MODE_VIDEO_HSE |
+			  MIPI_DSI_CLOCK_NON_CONTINUOUS;
+
+	drm_panel_init(&ctx->panel, dev, &hx83112a_panel_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+	ctx->panel.prepare_prev_first = true;
+
+	ret = drm_panel_of_backlight(&ctx->panel);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get backlight\n");
+
+	drm_panel_add(&ctx->panel);
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0) {
+		dev_err_probe(dev, ret, "Failed to attach to DSI host\n");
+		drm_panel_remove(&ctx->panel);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void hx83112a_remove(struct mipi_dsi_device *dsi)
+{
+	struct hx83112a_panel *ctx = mipi_dsi_get_drvdata(dsi);
+	int ret;
+
+	ret = mipi_dsi_detach(dsi);
+	if (ret < 0)
+		dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret);
+
+	drm_panel_remove(&ctx->panel);
+}
+
+static const struct of_device_id hx83112a_of_match[] = {
+	{ .compatible = "fairphone,fp4-hx83112a-djn" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, hx83112a_of_match);
+
+static struct mipi_dsi_driver hx83112a_driver = {
+	.probe = hx83112a_probe,
+	.remove = hx83112a_remove,
+	.driver = {
+		.name = "panel-himax-hx83112a",
+		.of_match_table = hx83112a_of_match,
+	},
+};
+module_mipi_dsi_driver(hx83112a_driver);
+
+MODULE_DESCRIPTION("DRM driver for hx83112a-equipped DSI panels");
+MODULE_LICENSE("GPL");