[v11,3/3] drm: exynos: dsi: Restore proper bridge chain order

Restore the proper bridge chain by finding the previous bridge
in the chain instead of passing NULL.

This establishes a proper bridge chain while attaching downstream
bridges.

Reviewed-by: Marek Vasut <marex@denx.de>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v11:
- add bridge.pre_enable_prev_first
Changes for v10:
- collect Marek review tag

 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
  

On 12.12.22 19:29, Jagan Teki wrote:
> Restore the proper bridge chain by finding the previous bridge
> in the chain instead of passing NULL.
> 
> This establishes a proper bridge chain while attaching downstream
> bridges.
> 
> Reviewed-by: Marek Vasut <marex@denx.de>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>

Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
  

Hi Jagan

Responding due to Marek's comment on the "Add Samsung MIPI DSIM
bridge" series, although I know very little about the Exynos
specifics, and may well be missing context of what you're trying to
achieve.

On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Restore the proper bridge chain by finding the previous bridge
> in the chain instead of passing NULL.
>
> This establishes a proper bridge chain while attaching downstream
> bridges.
>
> Reviewed-by: Marek Vasut <marex@denx.de>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v11:
> - add bridge.pre_enable_prev_first
> Changes for v10:
> - collect Marek review tag
>
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index ec673223d6b7..9d10a89d28f1 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge,
>  {
>         struct exynos_dsi *dsi = bridge_to_dsi(bridge);
>
> -       return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags);
> +       return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge,
> +                                flags);

Agreed on this change.

>  }
>
>  static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = {
> @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>
>         drm_bridge_add(&dsi->bridge);
>
> -       drm_bridge_attach(encoder, &dsi->bridge, NULL, 0);
> +       drm_bridge_attach(encoder, &dsi->bridge,
> +                         list_first_entry_or_null(&encoder->bridge_chain,
> +                                                  struct drm_bridge,
> +                                                  chain_node), 0);

What bridge are you expecting between the encoder and this bridge?
The encoder is the drm_simple_encoder_init encoder that you've created
in exynos_dsi_bind, so separating that from the bridge you're also
creating here seems weird.

>
>         /*
>          * This is a temporary solution and should be made by more generic way.
> @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>         dsi->bridge.funcs = &exynos_dsi_bridge_funcs;
>         dsi->bridge.of_node = dev->of_node;
>         dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
> +       dsi->bridge.pre_enable_prev_first = true;

Setting dsi->bridge.pre_enable_prev_first on what is presumably the
DSI host controller seems a little odd.
Same question again - what bridge are you expecting to be upstream of
the DSI host that needs to be preenabled before it? Whilst it's
possible that there's another bridge, I'd have expected that to be the
first link from your encoder as they appear to both belong to the same
bit of driver.

  Dave

>         ret = component_add(dev, &exynos_dsi_component_ops);
>         if (ret)
> --
> 2.25.1
>
  

Hi Dave,

On Sat, Jan 21, 2023 at 12:26 AM Dave Stevenson
<dave.stevenson@raspberrypi.com> wrote:
>
> Hi Jagan
>
> Responding due to Marek's comment on the "Add Samsung MIPI DSIM
> bridge" series, although I know very little about the Exynos
> specifics, and may well be missing context of what you're trying to
> achieve.
>
> On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > Restore the proper bridge chain by finding the previous bridge
> > in the chain instead of passing NULL.
> >
> > This establishes a proper bridge chain while attaching downstream
> > bridges.
> >
> > Reviewed-by: Marek Vasut <marex@denx.de>
> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> > Changes for v11:
> > - add bridge.pre_enable_prev_first
> > Changes for v10:
> > - collect Marek review tag
> >
> >  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > index ec673223d6b7..9d10a89d28f1 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge,
> >  {
> >         struct exynos_dsi *dsi = bridge_to_dsi(bridge);
> >
> > -       return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags);
> > +       return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge,
> > +                                flags);
>
> Agreed on this change.
>
> >  }
> >
> >  static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = {
> > @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
> >
> >         drm_bridge_add(&dsi->bridge);
> >
> > -       drm_bridge_attach(encoder, &dsi->bridge, NULL, 0);
> > +       drm_bridge_attach(encoder, &dsi->bridge,
> > +                         list_first_entry_or_null(&encoder->bridge_chain,
> > +                                                  struct drm_bridge,
> > +                                                  chain_node), 0);
>
> What bridge are you expecting between the encoder and this bridge?
> The encoder is the drm_simple_encoder_init encoder that you've created
> in exynos_dsi_bind, so separating that from the bridge you're also
> creating here seems weird.
>
> >
> >         /*
> >          * This is a temporary solution and should be made by more generic way.
> > @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev)
> >         dsi->bridge.funcs = &exynos_dsi_bridge_funcs;
> >         dsi->bridge.of_node = dev->of_node;
> >         dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
> > +       dsi->bridge.pre_enable_prev_first = true;
>
> Setting dsi->bridge.pre_enable_prev_first on what is presumably the
> DSI host controller seems a little odd.
> Same question again - what bridge are you expecting to be upstream of
> the DSI host that needs to be preenabled before it? Whilst it's
> possible that there's another bridge, I'd have expected that to be the
> first link from your encoder as they appear to both belong to the same
> bit of driver.

Let me answer all together here. I can explain a bit about one of the
pipelines used in Exynos. Exynos DSI DRM drivers have some strict host
initialization which is not the same as what we used in i.MX8M even
though it uses the same DSIM IP.

Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel

Here MIC is the bridge, Exynos DSI is the bridge and the requirement
is to expect the upstream bridge to pre_enable first from DSI which
means the MIC.

Jagan.
  

Hi Jagan

On Fri, 20 Jan 2023 at 19:10, Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Hi Dave,
>
> On Sat, Jan 21, 2023 at 12:26 AM Dave Stevenson
> <dave.stevenson@raspberrypi.com> wrote:
> >
> > Hi Jagan
> >
> > Responding due to Marek's comment on the "Add Samsung MIPI DSIM
> > bridge" series, although I know very little about the Exynos
> > specifics, and may well be missing context of what you're trying to
> > achieve.
> >
> > On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > Restore the proper bridge chain by finding the previous bridge
> > > in the chain instead of passing NULL.
> > >
> > > This establishes a proper bridge chain while attaching downstream
> > > bridges.
> > >
> > > Reviewed-by: Marek Vasut <marex@denx.de>
> > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > > Changes for v11:
> > > - add bridge.pre_enable_prev_first
> > > Changes for v10:
> > > - collect Marek review tag
> > >
> > >  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > > index ec673223d6b7..9d10a89d28f1 100644
> > > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > > @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge,
> > >  {
> > >         struct exynos_dsi *dsi = bridge_to_dsi(bridge);
> > >
> > > -       return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags);
> > > +       return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge,
> > > +                                flags);
> >
> > Agreed on this change.
> >
> > >  }
> > >
> > >  static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = {
> > > @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
> > >
> > >         drm_bridge_add(&dsi->bridge);
> > >
> > > -       drm_bridge_attach(encoder, &dsi->bridge, NULL, 0);
> > > +       drm_bridge_attach(encoder, &dsi->bridge,
> > > +                         list_first_entry_or_null(&encoder->bridge_chain,
> > > +                                                  struct drm_bridge,
> > > +                                                  chain_node), 0);
> >
> > What bridge are you expecting between the encoder and this bridge?
> > The encoder is the drm_simple_encoder_init encoder that you've created
> > in exynos_dsi_bind, so separating that from the bridge you're also
> > creating here seems weird.
> >
> > >
> > >         /*
> > >          * This is a temporary solution and should be made by more generic way.
> > > @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev)
> > >         dsi->bridge.funcs = &exynos_dsi_bridge_funcs;
> > >         dsi->bridge.of_node = dev->of_node;
> > >         dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
> > > +       dsi->bridge.pre_enable_prev_first = true;
> >
> > Setting dsi->bridge.pre_enable_prev_first on what is presumably the
> > DSI host controller seems a little odd.
> > Same question again - what bridge are you expecting to be upstream of
> > the DSI host that needs to be preenabled before it? Whilst it's
> > possible that there's another bridge, I'd have expected that to be the
> > first link from your encoder as they appear to both belong to the same
> > bit of driver.
>
> Let me answer all together here. I can explain a bit about one of the
> pipelines used in Exynos. Exynos DSI DRM drivers have some strict host
> initialization which is not the same as what we used in i.MX8M even
> though it uses the same DSIM IP.
>
> Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel
>
> Here MIC is the bridge, Exynos DSI is the bridge and the requirement
> is to expect the upstream bridge to pre_enable first from DSI which
> means the MIC.

That makes sense for the pre_enable_prev_first flag.

The drm_bridge_attach(... list_first_entry_or_null) still seems a
little weird. I think you are making the assumption that there is only
ever going to be the zero or one bridge (the MIC) between encoder and
DSI bridge - the DSI bridge is linking itself to the first entry off
the encoder bridge_chain (or NULL to link to the encoder). Is that
reasonable? I've no idea!

I must confess to not having looked at the attaching sequence
recently, and I'm about to head home for the weekend.
I have no real knowledge of how Exynos is working, and am aware that
you're having to rejuggle stuff to try and support i.MX8M and Exynos,
so leave that one up to you.

Cheers
  Dave
  

On 20.01.23 20:42, Dave Stevenson wrote:
> Hi Jagan
> 
> On Fri, 20 Jan 2023 at 19:10, Jagan Teki <jagan@amarulasolutions.com> wrote:
>>
>> Hi Dave,
>>
>> On Sat, Jan 21, 2023 at 12:26 AM Dave Stevenson
>> <dave.stevenson@raspberrypi.com> wrote:
>>>
>>> Hi Jagan
>>>
>>> Responding due to Marek's comment on the "Add Samsung MIPI DSIM
>>> bridge" series, although I know very little about the Exynos
>>> specifics, and may well be missing context of what you're trying to
>>> achieve.
>>>
>>> On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote:
>>>>
>>>> Restore the proper bridge chain by finding the previous bridge
>>>> in the chain instead of passing NULL.
>>>>
>>>> This establishes a proper bridge chain while attaching downstream
>>>> bridges.
>>>>
>>>> Reviewed-by: Marek Vasut <marex@denx.de>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>>> ---
>>>> Changes for v11:
>>>> - add bridge.pre_enable_prev_first
>>>> Changes for v10:
>>>> - collect Marek review tag
>>>>
>>>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++--
>>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>> index ec673223d6b7..9d10a89d28f1 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>> @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge,
>>>>  {
>>>>         struct exynos_dsi *dsi = bridge_to_dsi(bridge);
>>>>
>>>> -       return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags);
>>>> +       return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge,
>>>> +                                flags);
>>>
>>> Agreed on this change.
>>>
>>>>  }
>>>>
>>>>  static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = {
>>>> @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>>>>
>>>>         drm_bridge_add(&dsi->bridge);
>>>>
>>>> -       drm_bridge_attach(encoder, &dsi->bridge, NULL, 0);
>>>> +       drm_bridge_attach(encoder, &dsi->bridge,
>>>> +                         list_first_entry_or_null(&encoder->bridge_chain,
>>>> +                                                  struct drm_bridge,
>>>> +                                                  chain_node), 0);
>>>
>>> What bridge are you expecting between the encoder and this bridge?
>>> The encoder is the drm_simple_encoder_init encoder that you've created
>>> in exynos_dsi_bind, so separating that from the bridge you're also
>>> creating here seems weird.
>>>
>>>>
>>>>         /*
>>>>          * This is a temporary solution and should be made by more generic way.
>>>> @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>>>>         dsi->bridge.funcs = &exynos_dsi_bridge_funcs;
>>>>         dsi->bridge.of_node = dev->of_node;
>>>>         dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
>>>> +       dsi->bridge.pre_enable_prev_first = true;
>>>
>>> Setting dsi->bridge.pre_enable_prev_first on what is presumably the
>>> DSI host controller seems a little odd.
>>> Same question again - what bridge are you expecting to be upstream of
>>> the DSI host that needs to be preenabled before it? Whilst it's
>>> possible that there's another bridge, I'd have expected that to be the
>>> first link from your encoder as they appear to both belong to the same
>>> bit of driver.
>>
>> Let me answer all together here. I can explain a bit about one of the
>> pipelines used in Exynos. Exynos DSI DRM drivers have some strict host
>> initialization which is not the same as what we used in i.MX8M even
>> though it uses the same DSIM IP.
>>
>> Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel
>>
>> Here MIC is the bridge, Exynos DSI is the bridge and the requirement
>> is to expect the upstream bridge to pre_enable first from DSI which
>> means the MIC.
> 
> That makes sense for the pre_enable_prev_first flag.
> 
> The drm_bridge_attach(... list_first_entry_or_null) still seems a
> little weird. I think you are making the assumption that there is only
> ever going to be the zero or one bridge (the MIC) between encoder and
> DSI bridge - the DSI bridge is linking itself to the first entry off
> the encoder bridge_chain (or NULL to link to the encoder). Is that
> reasonable? I've no idea!

I think the assumption is reasonable for now, as it covers both types of
chains this driver is currently used in (Exynos and i.MX). And IIUC this
change is mainly needed for (backward) compatibility with the somewhat
"special" chain in Exynos.

> 
> I must confess to not having looked at the attaching sequence
> recently, and I'm about to head home for the weekend.
> I have no real knowledge of how Exynos is working, and am aware that
> you're having to rejuggle stuff to try and support i.MX8M and Exynos,
> so leave that one up to you.

I strongly vote for leaving this as is for now. We already spent too
much time finding a satisfying solution that covers Exynos and i.MX.
There might be room for improvements in the future, but in my opinion
this is good enough to get merged.
  

Hi Dave,

On Sat, Jan 21, 2023 at 1:12 AM Dave Stevenson
<dave.stevenson@raspberrypi.com> wrote:
>
> Hi Jagan
>
> On Fri, 20 Jan 2023 at 19:10, Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > Hi Dave,
> >
> > On Sat, Jan 21, 2023 at 12:26 AM Dave Stevenson
> > <dave.stevenson@raspberrypi.com> wrote:
> > >
> > > Hi Jagan
> > >
> > > Responding due to Marek's comment on the "Add Samsung MIPI DSIM
> > > bridge" series, although I know very little about the Exynos
> > > specifics, and may well be missing context of what you're trying to
> > > achieve.
> > >
> > > On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote:
> > > >
> > > > Restore the proper bridge chain by finding the previous bridge
> > > > in the chain instead of passing NULL.
> > > >
> > > > This establishes a proper bridge chain while attaching downstream
> > > > bridges.
> > > >
> > > > Reviewed-by: Marek Vasut <marex@denx.de>
> > > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > ---
> > > > Changes for v11:
> > > > - add bridge.pre_enable_prev_first
> > > > Changes for v10:
> > > > - collect Marek review tag
> > > >
> > > >  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++--
> > > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > > > index ec673223d6b7..9d10a89d28f1 100644
> > > > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > > > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > > > @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge,
> > > >  {
> > > >         struct exynos_dsi *dsi = bridge_to_dsi(bridge);
> > > >
> > > > -       return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags);
> > > > +       return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge,
> > > > +                                flags);
> > >
> > > Agreed on this change.
> > >
> > > >  }
> > > >
> > > >  static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = {
> > > > @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
> > > >
> > > >         drm_bridge_add(&dsi->bridge);
> > > >
> > > > -       drm_bridge_attach(encoder, &dsi->bridge, NULL, 0);
> > > > +       drm_bridge_attach(encoder, &dsi->bridge,
> > > > +                         list_first_entry_or_null(&encoder->bridge_chain,
> > > > +                                                  struct drm_bridge,
> > > > +                                                  chain_node), 0);
> > >
> > > What bridge are you expecting between the encoder and this bridge?
> > > The encoder is the drm_simple_encoder_init encoder that you've created
> > > in exynos_dsi_bind, so separating that from the bridge you're also
> > > creating here seems weird.
> > >
> > > >
> > > >         /*
> > > >          * This is a temporary solution and should be made by more generic way.
> > > > @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev)
> > > >         dsi->bridge.funcs = &exynos_dsi_bridge_funcs;
> > > >         dsi->bridge.of_node = dev->of_node;
> > > >         dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
> > > > +       dsi->bridge.pre_enable_prev_first = true;
> > >
> > > Setting dsi->bridge.pre_enable_prev_first on what is presumably the
> > > DSI host controller seems a little odd.
> > > Same question again - what bridge are you expecting to be upstream of
> > > the DSI host that needs to be preenabled before it? Whilst it's
> > > possible that there's another bridge, I'd have expected that to be the
> > > first link from your encoder as they appear to both belong to the same
> > > bit of driver.
> >
> > Let me answer all together here. I can explain a bit about one of the
> > pipelines used in Exynos. Exynos DSI DRM drivers have some strict host
> > initialization which is not the same as what we used in i.MX8M even
> > though it uses the same DSIM IP.
> >
> > Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel
> >
> > Here MIC is the bridge, Exynos DSI is the bridge and the requirement
> > is to expect the upstream bridge to pre_enable first from DSI which
> > means the MIC.
>
> That makes sense for the pre_enable_prev_first flag.
>
> The drm_bridge_attach(... list_first_entry_or_null) still seems a
> little weird. I think you are making the assumption that there is only
> ever going to be the zero or one bridge (the MIC) between encoder and
> DSI bridge - the DSI bridge is linking itself to the first entry off
> the encoder bridge_chain (or NULL to link to the encoder). Is that
> reasonable? I've no idea!

That is true, the reason would be that DSI bridges still use an
encoder, usually, the first bridge in the chain will use an encoder
and subsequent bridges are fully bridge-driven drivers (without an
encoder) in order to follow the DRM bridge chain topology.
Unfortunately, the Exynos DRM drivers follow component-based binding
so DSI is part of that topology any attempt to exclude the encoder
from it not compatible with Exynos DRM drivers. So, in order to make
the bridge chain work properly we assume that linking. I think this
bridge attach from the DSI driver bind can be dropped if we make
Exynos DSIM as an independent bridge driver.

Jagan.