Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 13 Oct 2022 14:58:29 +0200
From:   Francesco Dolcini <francesco@dolcini.it>
To:     Max Krummenacher <max.oss.09@gmail.com>,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        Marek Vasut <marex@denx.de>
Cc:     max.krummenacher@toradex.com,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Rob Herring <robh@kernel.org>,
        Maxime Ripard <mripard@kernel.org>,
        Christoph Niedermaier <cniedermaier@dh-electronics.com>,
        Francesco Dolcini <francesco.dolcini@toradex.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        David Airlie <airlied@linux.ie>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Sam Ravnborg <sam@ravnborg.org>,
        Thierry Reding <thierry.reding@gmail.com>,
        devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 2/4] dt-bindings: display: add new bus-format property
 for panel-dpi
Message-ID: <Y0gLdQleE64FQgn9@gaggiata.pivistrello.it>
Reply-To: francesco.dolcini@toradex.com
References: <20220628181838.2031-1-max.oss.09@gmail.com>
 <20220628181838.2031-3-max.oss.09@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220628181838.2031-3-max.oss.09@gmail.com>
Precedence: bulk

Hello Max, Marek, Dave et al.

On Tue, Jun 28, 2022 at 08:18:36PM +0200, Max Krummenacher wrote:
> From: Max Krummenacher <max.krummenacher@toradex.com>
> 
> The property is used to set the enum bus_format and infer the bpc
> for a panel defined by 'panel-dpi'.
> This specifies how the panel is connected to the display interface.
> 
> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> 

<snip>

> diff --git a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml
> index dae0676b5c6e..52f5db03b6a8 100644
> --- a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml
> @@ -26,7 +26,28 @@ properties:
>    height-mm: true
>    label: true
>    panel-timing: true
> -  port: true
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    description:
> +      Input port node, receives the panel data.
> +
> +    properties:
> +      endpoint:
> +        $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +
> +        properties:
> +          bus-format:
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            minimum: 0x1001
> +            maximum: 0x1fff
> +            description: |
> +              Describes how the display panel is connected to the display interface.
> +              Valid values are defined in <dt-bindings/display/dt-media-bus-format.h>.
> +              The mapping between the color/significance of the panel lines to the
> +              parallel data lines are defined in:
> +              https://www.kernel.org/doc/html/v5.17/userspace-api/media/v4l/subdev-formats.html#packed-rgb-formats
> +

Last month I had the chance to talk in person about this topic with
Dave, Marek and Max in Dublin.

My understanding is that this change is addressing a general need, Dave
confirmed me they have a downstream patch for raspberrypi [1].

From what I could tell the only concern is about the actual encoding of
this `bus-format` property.

I am personally convinced that a simple enum is the way to go, I think
that Marek proposal is adding complexity and not flexibility (from my
understanding Dave is on the same page, just correct me if I
misunderstood you).

The current proposal is already encoding the exact bit placing as
described in Documentation/userspace-api/media/v4l/subdev-formats.rst [2],
this enumeration can be extended to address any future needs
and I would not invent a new one to define the exact same
things (and using the same enum was also suggested by Rob).

Marek: you told me that you had some concern about some valid use case
not covered by this solution, would you mind explaining why that would
not be covered with an addition on this enumeration?

Any other opinion on this topic? How can we move this forward?

Francesco

[1] https://github.com/raspberrypi/linux/commit/8e43f1898191b43aa7ed6e6ca3a4cd28709af86d
[2] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/subdev-formats.html