On Thu, Mar 30, 2023 at 12:15:49PM +0530, Jagan Teki wrote: > 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? Yes, and given the current state of the DCS-in-HS discussion, I even think it's does more harm than good. Maxime