On Wed, Mar 29, 2023 at 9:36 PM Maxime Ripard <maxime@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx>
> > >
> > > [...]
> > >
> > > > +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:
Correct.
>
> - You modify the existing driver to be a bridge
Yes, Convert to bridge driver - register drm_bridge_add and replace
encoder ops with bridge ops.
>
> - And you support downstream device being bridges.
Yes, Support the downstream bridge. (If I'm correct we can still use
encoder ops with this).
If we see the hierarchy of support it would
1. support the downstream bridge.
2. convert to the bridge driver.
>
> 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.
Do you mean converting to bridge driver is not needed?
Thanks,
Jagan.
