Re: [PATCH v1 1/3] dt-bindings: display: st7701: Add Anbernic RG28XX panel

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

 



Hello Krzysztof,

Thank you for your reply!

On Tue, Jun 18, 2024 at 6:17 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote:
>
> On 18/06/2024 10:15, Hironori KIKUCHI wrote:
> > The RG28XX panel is a panel specific to the Anbernic RG28XX.
> > It is 2.8 inches in size (diagonally) with a resolution of 480x640.
> >
> > Signed-off-by: Hironori KIKUCHI <kikuchan98@xxxxxxxxx>
> > ---
> >  .../display/panel/sitronix,st7701.yaml        | 36 +++++++++++++++++--
> >  1 file changed, 34 insertions(+), 2 deletions(-)
>
> Nothing explains in the commit msg why rg28xx is actually st7701.
> Changing interface to SPI suggests it is not.

Thanks, I'll explain like this;
---
dt-bindings: display: st7701: Add Anbernic RG28XX panel

The RG28XX panel is a panel specific to the Anbernic RG28XX
handheld device. It is 2.8 inches in size (diagonally) with a
resolution of 480x640.

This panel is driven by a variant of ST7701 driver IC internally,
confirmed by dumping and analyzing its BSP initialization sequence
by using a logic analyzer. It is very similar to the existing
densitron,dmt028vghmcmi-1a panel, but differs in some unknown
register values, so add a new entry for the panel to distinguish them.

Additionally, it is connected over SPI, instead of MIPI DSI. So
add and modify for SPI as well.
---

>
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/sitronix,st7701.yaml b/Documentation/devicetree/bindings/display/panel/sitronix,st7701.yaml
> > index b348f5bf0a9..04f6751ccca 100644
> > --- a/Documentation/devicetree/bindings/display/panel/sitronix,st7701.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/sitronix,st7701.yaml
> > @@ -22,19 +22,21 @@ description: |
> >
> >  allOf:
> >    - $ref: panel-common.yaml#
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> >
> >  properties:
> >    compatible:
> >      items:
> >        - enum:
> >            - anbernic,rg-arc-panel
> > +          - anbernic,rg28xx-panel
>
> What is xx? Wildcards are not allowed, in general.
>
> Can it be anything else than panel? If not, then drop "-panel".

It's supprising but it actually is a product name of the handheld device...
The panel comes with the device, and part# is completely unknown.

>
>
> >            - densitron,dmt028vghmcmi-1a
> >            - elida,kd50t048a
> >            - techstar,ts8550b
> >        - const: sitronix,st7701
> >
> >    reg:
> > -    description: DSI virtual channel used by that screen
> > +    description: DSI / SPI channel used by that screen
> >      maxItems: 1
> >
> >    VCC-supply:
> > @@ -43,6 +45,13 @@ properties:
> >    IOVCC-supply:
> >      description: I/O system regulator
> >
> > +  dc-gpios:
> > +    maxItems: 1
> > +    description: |
>
> Do not need '|' unless you need to preserve formatting.

Thanks, I'll remove it.

>
> > +      Controller data/command selection (D/CX) in 4-line SPI mode.
> > +      If not set, the controller is in 3-line SPI mode.
> > +      No effect for DSI.
>
> Which devices can be connected over SPI? It seems not all, so this
> should be disallowed (": false" in allOf:if:then:; move the allOf to
> bottom like in example-schema) for them.

Hmm... That's a difficult question...

There are 3 types of connection that trying to support:
DSI, SPI with D/CX pin, and SPI without D/CX pin.

The dc-gpios is required for SPI with D/CX pin, but not for others.

DSI:
- anbernic,rg-arc-panel
- densitron,dmt028vghmcmi-1a
- elida,kd50t048a
- techstar,ts8550b

SPI without D/CX pin:
- anbernic,rg28xx-panel

But, there are no panels with D/CX pin so far.
How should I deal with this? just disallow all, perhaps?


BTW, does panel's compatible string consider to include it's interface?
ie, what if two panels use the exact same commands and timings, but
over different interface,
... are they "compatible" or not?

>
> Best regards,
> Krzysztof
>

Best regards,
kikuchan.




[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