From: Rob Herring <robh@kernel.org>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Robert Foss <robert.foss@linaro.org>
Cc: David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
Matthias Brugger <matthias.bgg@gmail.com>,
Xin Ji <xji@analogixsemi.com>, Hsin-Yi Wang <hsinyi@chromium.org>,
dri-devel <dri-devel@lists.freedesktop.org>,
devicetree@vger.kernel.org,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
"moderated list:ARM/Mediatek SoC support"
<linux-mediatek@lists.infradead.org>,
Chen-Yu Tsai <wenst@chromium.org>
Subject: Re: [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define"
Date: Mon, 7 Mar 2022 11:32:35 -0600 [thread overview]
Message-ID: <CAL_Jsq+AqKJ7hU3j4sEf_61ibiyJqOWocc43B0q2G4fHDxPi1g@mail.gmail.com> (raw)
In-Reply-To: <YiY8ko5WX3mQfDLY@pendragon.ideasonboard.com>
On Mon, Mar 7, 2022 at 11:11 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Mar 07, 2022 at 05:57:47PM +0100, Robert Foss wrote:
> > On Mon, 7 Mar 2022 at 17:38, Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Mon, Mar 07, 2022 at 04:45:57PM +0100, Robert Foss wrote:
> > > > This reverts commit a43661e7e819b100e1f833a35018560a1d9abb39.
> > >
> > > S-o-b and reason for the revert?
> > >
> > > > ---
> > > > .../display/bridge/analogix,anx7625.yaml | 65 +------------------
> > > > 1 file changed, 2 insertions(+), 63 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > index 1d3e88daca041..ab48ab2f4240d 100644
> > > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > @@ -43,70 +43,14 @@ properties:
> > > > vdd33-supply:
> > > > description: Regulator that provides the supply 3.3V power.
> > > >
> > > > - analogix,lane0-swing:
> > > > - $ref: /schemas/types.yaml#/definitions/uint8-array
> > > > - minItems: 1
> > > > - maxItems: 20
> > > > - description:
> > > > - an array of swing register setting for DP tx lane0 PHY.
> > > > - Registers 0~9 are Swing0_Pre0, Swing1_Pre0, Swing2_Pre0,
> > > > - Swing3_Pre0, Swing0_Pre1, Swing1_Pre1, Swing2_Pre1, Swing0_Pre2,
> > > > - Swing1_Pre2, Swing0_Pre3, they are for [Boost control] and
> > > > - [Swing control] setting.
> > > > - Registers 0~9, bit 3:0 is [Boost control], these bits control
> > > > - post cursor manual, increase the [Boost control] to increase
> > > > - Pre-emphasis value.
> > > > - Registers 0~9, bit 6:4 is [Swing control], these bits control
> > > > - swing manual, increase [Swing control] setting to add Vp-p value
> > > > - for each Swing, Pre.
> > > > - Registers 10~19 are Swing0_Pre0, Swing1_Pre0, Swing2_Pre0,
> > > > - Swing3_Pre0, Swing0_Pre1, Swing1_Pre1, Swing2_Pre1, Swing0_Pre2,
> > > > - Swing1_Pre2, Swing0_Pre3, they are for [R select control] and
> > > > - [R Termination control] setting.
> > > > - Registers 10~19, bit 4:0 is [R select control], these bits are
> > > > - compensation manual, increase it can enhance IO driven strength
> > > > - and Vp-p.
> > > > - Registers 10~19, bit 5:6 is [R termination control], these bits
> > > > - adjust 50ohm impedance of DP tx termination. 00:55 ohm,
> > > > - 01:50 ohm(default), 10:45 ohm, 11:40 ohm.
> > > > -
> > > > - analogix,lane1-swing:
> > > > - $ref: /schemas/types.yaml#/definitions/uint8-array
> > > > - minItems: 1
> > > > - maxItems: 20
> > > > - description:
> > > > - an array of swing register setting for DP tx lane1 PHY.
> > > > - DP TX lane1 swing register setting same with lane0
> > > > - swing, please refer lane0-swing property description.
> > >
> > > These apply to the DP side, so no need to revert this part.
> >
> > Ack.
> >
> > >
> > > > -
> > > > - analogix,audio-enable:
> > > > - type: boolean
> > > > - description: let the driver enable audio HDMI codec function or not.
> > > > -
> > >
> > > Not sure on this one...
> >
> > These additions are independent from my reading of this, would you
> > like a v2 with only the bus-type related changes reverted?
Yes.
> >
> > >
> > > > ports:
> > > > $ref: /schemas/graph.yaml#/properties/ports
> > > >
> > > > properties:
> > > > port@0:
> > > > - $ref: /schemas/graph.yaml#/$defs/port-base
> > > > - unevaluatedProperties: false
> > > > + $ref: /schemas/graph.yaml#/properties/port
> > > > description:
> > > > - MIPI DSI/DPI input.
> > > > -
> > > > - properties:
> > > > - endpoint:
> > > > - $ref: /schemas/media/video-interfaces.yaml#
> > > > - type: object
> > > > - additionalProperties: false
> > > > -
> > > > - properties:
> > > > - remote-endpoint: true
> > > > -
> > > > - bus-type:
> > > > - enum: [1, 5]
> > >
> > > I think the error here is really 1 should be 4 which corresponds to
> > > D-PHY which is used by both CSI and DSI. Otherwise, I don't really see
> > > the issue with bus-type being shared between CSI and DSI.
> >
> > I think that would be a correct solution. And ignoring everything
> > else, the range of this property is something that should be fixed.
> >
> > But that would mean that CPI (camera parallel interface) and DPI
> > (display parallel interface) would share the
> > V4L2_FWNODE_BUS_TYPE_PARALLEL enum. I think that would be perfectly
> > functional, but it is not what V4L2_FWNODE_BUS_TYPE_PARALLEL is
> > documented to represent. As far as I can see it's only intended to
> > represent CPI.
>
> Are you aware of any standard documenting camera parallel interfaces
> with separate sync signals ? I was looking for that the other day, and
> couldn't find much. CPI seems to be an old MIPI standard, but I couldn't
> find any public document, I'not not sure if it actually matches.
I don't recall. Generally, I think the camera side is not quite the
mess the display side is with all the formats.
> Another common parallel interface in SoCs is AXI4 Stream, which we will
> likely need a bus type for. We'll then have to decide on how to handle
> on-SoC custom parallel buses.
Except for those maybe.
> > Instead of having V4L2_FWNODE_BUS_TYPE_PARALLEL represent two
> > standards, I think they should be split. And possibly
> > V4L2_FWNODE_BUS_TYPE_PARALLEL should be renamed for CPI, but that is a
> > separate story. This would provide for the neatest and most legible
> > solution. If this solution is implemented, this range would be
> > incorrect. Additionally the snippet reverted in 2/2 of this series
> > would no longer be valid.
> >
> > As it stands V4L2_FWNODE_BUS_TYPE_PARALLEL was used to represent DPI
> > due to not being caught in the review process.
>
> We may end up using those values, but I think it should be discussed and
> not rushed in v5.17 if possible.
Given it's not actually used (correctly), agreed.
Rob
next prev parent reply other threads:[~2022-03-07 17:32 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-07 15:45 [PATCH v1 0/2] Revert vendor property from anx7625 bindings Robert Foss
2022-03-07 15:45 ` [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define" Robert Foss
2022-03-07 16:20 ` Robert Foss
2022-03-07 16:38 ` Rob Herring
2022-03-07 16:57 ` Robert Foss
2022-03-07 17:10 ` Laurent Pinchart
2022-03-07 17:32 ` Rob Herring [this message]
2022-03-07 15:45 ` [PATCH v1 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint" Robert Foss
2022-03-07 16:20 ` Robert Foss
2022-03-08 3:47 ` Chen-Yu Tsai
2022-03-07 16:43 ` Rob Herring
2022-03-07 15:51 ` [PATCH v1 0/2] Revert vendor property from anx7625 bindings Laurent Pinchart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAL_Jsq+AqKJ7hU3j4sEf_61ibiyJqOWocc43B0q2G4fHDxPi1g@mail.gmail.com \
--to=robh@kernel.org \
--cc=airlied@linux.ie \
--cc=daniel@ffwll.ch \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=hsinyi@chromium.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=robert.foss@linaro.org \
--cc=wenst@chromium.org \
--cc=xji@analogixsemi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).