Re: [PATCH v7 12/12] drm: sun4: dsi: Convert to bridge driver

From: Maxime Ripard <maxime@cerno.tech>
To: Jagan Teki <jagan@amarulasolutions.com>
Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
	Andrzej Hajda <andrzej.hajda@intel.com>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Robert Foss <rfoss@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	Samuel Holland <samuel@sholland.org>,
	Sam Ravnborg <sam@ravnborg.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev, devicetree@vger.kernel.org,
	dri-devel@lists.freedesktop.org, Marek Vasut <marex@denx.de>,
	linux-amarula <linux-amarula@amarulasolutions.com>
Subject: Re: [PATCH v7 12/12] drm: sun4: dsi: Convert to bridge driver
Date: Wed, 29 Mar 2023 18:06:52 +0200	[thread overview]
Message-ID: <20230329160652.7gel5qvckzwihjx4@penduick> (raw)
In-Reply-To: <CAMty3ZDWK0xVe7E+gER+TihHf1yv3YAWgZc1GCJQ2V5KD_mN-g@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 2081 bytes --]

On Wed, Mar 29, 2023 at 09:08:17PM +0530, Jagan Teki wrote:
> On Wed, Mar 29, 2023 at 8:29 PM Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > Hi,
> >
> > The patch prefix should be drm/sun4i:
> 
> I did follow my previous prefix, I will update this.
> 
> >
> > On Wed, Mar 29, 2023 at 06:49:29PM +0530, Jagan Teki wrote:
> > > Convert the encoder to bridge driver in order to standardize on a
> > > single API by supporting all varients of downstream bridge devices.
> >
> > Which variant, and why do we need to convert to a bridge to support all of them?
> 
> Downstream bridge variants like DSI panel, DSI bridge and
> I2C-Configured DSI bridges. Bridge conversion would be required for
> the DSI host to access the more variety and complex downstream bridges
> in a standardized bridge chain way which is indeed complex for encoder
> driven DSI hosts.
> 
> >
> > > The drm_encoder can't be removed as it's exposed to userspace, so it
> > > then becomes a dumb encoder, without any operation implemented.
> > >
> > > Tested on DSI Panel, DSI Bridge, I2C-Configured DSI Bridge.
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >
> > [...]
> >
> > > +static const struct component_ops sun6i_dsi_ops;
> > > +
> > >  static int sun6i_dsi_attach(struct mipi_dsi_host *host,
> > >                           struct mipi_dsi_device *device)
> > >  {
> > >       struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
> > > -     struct drm_panel *panel = of_drm_find_panel(device->dev.of_node);
> >
> > That one looks unrelated. Why do you need that change?
> 
> This was replaced with drmm_of_dsi_get_bridge for lookup of both panel
> and bridge. I think I will separate this into another patch.

So, it looks to me that you're doing two (unrelated) things in that patch:

  - You modify the existing driver to be a bridge

  - And you support downstream device being bridges.

Both are orthogonal, can (and should!) be done separately, and I'm
pretty sure you don't actually need to do the former at all.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel