Re: [PATCH V2 1/2] dt-bindings: display: newvision,nv3051d: Add Anbernic 351V Support

[Chris Morgan <macroalpha82@gmail.com>]

On Fri, Aug 11, 2023 at 09:41:48AM -0500, Chris Morgan wrote:
> On Thu, Aug 10, 2023 at 05:24:09PM -0600, Rob Herring wrote:
> > On Wed, Aug 09, 2023 at 10:39:40AM -0500, Chris Morgan wrote:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > Document the Anbernic RG351V panel, which appears to be identical to
> > > the panel used in their 353 series except for in inclusion of an
> > > additional DSI format flag.
> > > 
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > ---
> > >  .../display/panel/newvision,nv3051d.yaml       | 18 ++++++++++--------
> > >  1 file changed, 10 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml
> > > index 116c1b6030a2..576f3640cb33 100644
> > > --- a/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml
> > > +++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml
> > > @@ -7,9 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
> > >  title: NewVision NV3051D based LCD panel
> > >  
> > >  description: |
> > > -  The NewVision NV3051D is a driver chip used to drive DSI panels. For now,
> > > -  this driver only supports the 640x480 panels found in the Anbernic RG353
> > > -  based devices.
> > > +  The NewVision NV3051D is a driver chip used to drive DSI panels.
> > >  
> > >  maintainers:
> > >    - Chris Morgan <macromorgan@hotmail.com>
> > > @@ -19,11 +17,15 @@ allOf:
> > >  
> > >  properties:
> > >    compatible:
> > > -    items:
> > > -      - enum:
> > > -          - anbernic,rg353p-panel
> > > -          - anbernic,rg353v-panel
> > > -      - const: newvision,nv3051d
> > > +    oneOf:
> > > +      - items:
> > > +          - enum:
> > > +              - anbernic,rg353p-panel
> > > +              - anbernic,rg353v-panel
> > > +          - const: newvision,nv3051d
> > > +
> > > +      - items:
> > > +          - const: anbernic,rg351v-panel
> > 
> > I don't understand. Is this panel not based on newvision,nv3051d? If 
> > not, then it probably should be a different binding. Lot's of panel 
> > bindings have similar properties.
> 
> It appears to be the same panel (or extremely similar, honestly I don't
> know because there are no external markings on it). However, this
> specific implementation seems to require MIPI_DSI_CLOCK_NON_CONTINUOUS,
> not using it prevents the panel from working. As for the existing panel
> MIPI_DSI_CLOCK_NON_CONTINUOUS stops it from working. The different
> binding essentially determines whether or not that flag is present, but
> otherwise everything else is identical.
> 
> Chris

I don't want to lose sight of this, but I am not sure how to proceed.
What I can do instead is change the compatible string inside the driver
from newvision,nv3051d to either anbernic,rg353p-panel or 
anbernic,rg351v-panel. Then, I can remove anbernic,rg353v-panel as an
enum and replace it with anbernic,rg351v-panel. The gist of this is
that we have a Newvision NV3051D panel that will still be supported by
this driver in 2 different configurations, the 353P (which is identical
to the 353V) and the 351V (which has different mode flags but is
otherwise identical).

So long story short would it work if I did this, and modified the
driver and all in-use devicetrees accordingly? To my knowledge this
panel is only in use on boards that I submitted so I can update all
those and test them.

      - enum:
          - anbernic,rg351v-panel
          - anbernic,rg353p-panel
      - const: newvision,nv3051d

Thank you,
Chris.

> 
> > 
> > Rob