Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
On 08.12.2022 12:32, Jagan Teki wrote:
> On Tue, Dec 6, 2022 at 2:32 PM Frieder Schrempf
> wrote:
>> On 05.12.22 16:37, Frieder Schrempf wrote:
>>> Hi Dave,
>>>
>>> On 05.12.22 16:20, Dave Stevenson wrote:
Hi Frieder
On Mon, 5 Dec 2022 at 07:30, Frieder Schrempf
wrote:
> On 02.12.22 15:55, Dave Stevenson wrote:
>> Hi Marek
>>
>> On Fri, 2 Dec 2022 at 12:21, Marek Vasut wrote:
>>> On 12/2/22 11:52, Marek Szyprowski wrote:
Hi,
Sorry for delay, I was on a sick leave last 2 weeks.
On 28.11.2022 15:43, Jagan Teki wrote:
> ,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut wrote:
>> On 11/23/22 21:09, Jagan Teki wrote:
>>> On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut wrote:
On 11/17/22 14:04, Marek Szyprowski wrote:
> On 17.11.2022 05:58, Marek Vasut wrote:
>> On 11/10/22 19:38, Jagan Teki wrote:
>>> DSI host initialization handling in previous exynos dsi driver
>>> has
>>> some pitfalls. It initializes the host during host transfer()
>>> hook
>>> that is indeed not the desired call flow for I2C and any other
>>> DSI
>>> configured downstream bridges.
>>>
>>> Host transfer() is usually triggered for downstream DSI panels
>>> or
>>> bridges and I2C-configured-DSI bridges miss these host
>>> initialization
>>> as these downstream bridges use bridge operations hooks like
>>> pre_enable,
>>> and enable in order to initialize or set up the host.
>>>
>>> This patch is trying to handle the host init handler to satisfy
>>> all
>>> downstream panels and bridges. Added the
>>> DSIM_STATE_REINITIALIZED state
>>> flag to ensure that host init is also done on first cmd
>>> transfer, this
>>> helps existing DSI panels work on exynos platform (form Marek
>>> Szyprowski).
>>>
>>> v8, v7, v6, v5:
>>> * none
>>>
>>> v4:
>>> * update init handling to ensure host init done on first cmd
>>> transfer
>>>
>>> v3:
>>> * none
>>>
>>> v2:
>>> * check initialized state in samsung_dsim_init
>>>
>>> v1:
>>> * keep DSI init in host transfer
>>>
>>> Signed-off-by: Marek Szyprowski
>>> Signed-off-by: Jagan Teki
>>> ---
>>>drivers/gpu/drm/bridge/samsung-dsim.c | 25
>>> +
>>>include/drm/bridge/samsung-dsim.h | 5 +++--
>>>2 files changed, 20 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
>>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> index bb1f45fd5a88..ec7e01ae02ea 100644
>>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> @@ -1234,12 +1234,17 @@ static void
>>> samsung_dsim_disable_irq(struct
>>> samsung_dsim *dsi)
>>>disable_irq(dsi->irq);
>>>}
>>>-static int samsung_dsim_init(struct samsung_dsim *dsi)
>>> +static int samsung_dsim_init(struct samsung_dsim *dsi,
>>> unsigned int
>>> flag)
>>>{
>>>const struct samsung_dsim_driver_data *driver_data =
>>> dsi->driver_data;
>>>+if (dsi->state & flag)
>>> +return 0;
>>> +
>>>samsung_dsim_reset(dsi);
>>> -samsung_dsim_enable_irq(dsi);
>>> +
>>> +if (!(dsi->state & DSIM_STATE_INITIALIZED))
>>> +samsung_dsim_enable_irq(dsi);
>>> if (driver_data->reg_values[RESET_TYPE] ==
>>> DSIM_FUNCRST)
>>>samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) -
>>> 1);
>>> @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
>>> samsung_dsim *dsi)
>>>samsung_dsim_set_phy_ctrl(dsi);
>>>samsung_dsim_init_link(dsi);
>>>+dsi->state |= flag;
>>> +
>>>return 0;
>>>}
>>>@@ -1269,6 +1276,10 @@ static void
>>> samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
>>>}
>>> dsi->state |= DSIM_STATE_ENABLED;
>>> +
>>> +ret =
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
On Tue, Dec 6, 2022 at 2:32 PM Frieder Schrempf
wrote:
>
> On 05.12.22 16:37, Frieder Schrempf wrote:
> > Hi Dave,
> >
> > On 05.12.22 16:20, Dave Stevenson wrote:
> >> Hi Frieder
> >>
> >> On Mon, 5 Dec 2022 at 07:30, Frieder Schrempf
> >> wrote:
> >>>
> >>> On 02.12.22 15:55, Dave Stevenson wrote:
> Hi Marek
>
> On Fri, 2 Dec 2022 at 12:21, Marek Vasut wrote:
> >
> > On 12/2/22 11:52, Marek Szyprowski wrote:
> >> Hi,
> >>
> >> Sorry for delay, I was on a sick leave last 2 weeks.
> >>
> >> On 28.11.2022 15:43, Jagan Teki wrote:
> >>> ,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut wrote:
> On 11/23/22 21:09, Jagan Teki wrote:
> > On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut wrote:
> >> On 11/17/22 14:04, Marek Szyprowski wrote:
> >>> On 17.11.2022 05:58, Marek Vasut wrote:
> On 11/10/22 19:38, Jagan Teki wrote:
> > DSI host initialization handling in previous exynos dsi driver
> > has
> > some pitfalls. It initializes the host during host transfer()
> > hook
> > that is indeed not the desired call flow for I2C and any other
> > DSI
> > configured downstream bridges.
> >
> > Host transfer() is usually triggered for downstream DSI panels
> > or
> > bridges and I2C-configured-DSI bridges miss these host
> > initialization
> > as these downstream bridges use bridge operations hooks like
> > pre_enable,
> > and enable in order to initialize or set up the host.
> >
> > This patch is trying to handle the host init handler to satisfy
> > all
> > downstream panels and bridges. Added the
> > DSIM_STATE_REINITIALIZED state
> > flag to ensure that host init is also done on first cmd
> > transfer, this
> > helps existing DSI panels work on exynos platform (form Marek
> > Szyprowski).
> >
> > v8, v7, v6, v5:
> > * none
> >
> > v4:
> > * update init handling to ensure host init done on first cmd
> > transfer
> >
> > v3:
> > * none
> >
> > v2:
> > * check initialized state in samsung_dsim_init
> >
> > v1:
> > * keep DSI init in host transfer
> >
> > Signed-off-by: Marek Szyprowski
> > Signed-off-by: Jagan Teki
> > ---
> > drivers/gpu/drm/bridge/samsung-dsim.c | 25
> > +
> > include/drm/bridge/samsung-dsim.h | 5 +++--
> > 2 files changed, 20 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> > b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index bb1f45fd5a88..ec7e01ae02ea 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -1234,12 +1234,17 @@ static void
> > samsung_dsim_disable_irq(struct
> > samsung_dsim *dsi)
> > disable_irq(dsi->irq);
> > }
> > -static int samsung_dsim_init(struct samsung_dsim *dsi)
> > +static int samsung_dsim_init(struct samsung_dsim *dsi,
> > unsigned int
> > flag)
> > {
> > const struct samsung_dsim_driver_data *driver_data =
> > dsi->driver_data;
> > +if (dsi->state & flag)
> > +return 0;
> > +
> > samsung_dsim_reset(dsi);
> > -samsung_dsim_enable_irq(dsi);
> > +
> > +if (!(dsi->state & DSIM_STATE_INITIALIZED))
> > +samsung_dsim_enable_irq(dsi);
> > if (driver_data->reg_values[RESET_TYPE] ==
> > DSIM_FUNCRST)
> > samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) -
> > 1);
> > @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
> > samsung_dsim *dsi)
> > samsung_dsim_set_phy_ctrl(dsi);
> > samsung_dsim_init_link(dsi);
> > +dsi->state |= flag;
> > +
> > return 0;
> > }
> > @@ -1269,6 +1276,10 @@ static void
> > samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
> > }
> > dsi->state |= DSIM_STATE_ENABLED;
> > +
> > +ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
>
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
On 05.12.22 16:37, Frieder Schrempf wrote:
> Hi Dave,
>
> On 05.12.22 16:20, Dave Stevenson wrote:
>> Hi Frieder
>>
>> On Mon, 5 Dec 2022 at 07:30, Frieder Schrempf
>> wrote:
>>>
>>> On 02.12.22 15:55, Dave Stevenson wrote:
Hi Marek
On Fri, 2 Dec 2022 at 12:21, Marek Vasut wrote:
>
> On 12/2/22 11:52, Marek Szyprowski wrote:
>> Hi,
>>
>> Sorry for delay, I was on a sick leave last 2 weeks.
>>
>> On 28.11.2022 15:43, Jagan Teki wrote:
>>> ,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut wrote:
On 11/23/22 21:09, Jagan Teki wrote:
> On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut wrote:
>> On 11/17/22 14:04, Marek Szyprowski wrote:
>>> On 17.11.2022 05:58, Marek Vasut wrote:
On 11/10/22 19:38, Jagan Teki wrote:
> DSI host initialization handling in previous exynos dsi driver has
> some pitfalls. It initializes the host during host transfer() hook
> that is indeed not the desired call flow for I2C and any other DSI
> configured downstream bridges.
>
> Host transfer() is usually triggered for downstream DSI panels or
> bridges and I2C-configured-DSI bridges miss these host
> initialization
> as these downstream bridges use bridge operations hooks like
> pre_enable,
> and enable in order to initialize or set up the host.
>
> This patch is trying to handle the host init handler to satisfy
> all
> downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED
> state
> flag to ensure that host init is also done on first cmd transfer,
> this
> helps existing DSI panels work on exynos platform (form Marek
> Szyprowski).
>
> v8, v7, v6, v5:
> * none
>
> v4:
> * update init handling to ensure host init done on first cmd
> transfer
>
> v3:
> * none
>
> v2:
> * check initialized state in samsung_dsim_init
>
> v1:
> * keep DSI init in host transfer
>
> Signed-off-by: Marek Szyprowski
> Signed-off-by: Jagan Teki
> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 25
> +
> include/drm/bridge/samsung-dsim.h | 5 +++--
> 2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> b/drivers/gpu/drm/bridge/samsung-dsim.c
> index bb1f45fd5a88..ec7e01ae02ea 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -1234,12 +1234,17 @@ static void
> samsung_dsim_disable_irq(struct
> samsung_dsim *dsi)
> disable_irq(dsi->irq);
> }
> -static int samsung_dsim_init(struct samsung_dsim *dsi)
> +static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned
> int
> flag)
> {
> const struct samsung_dsim_driver_data *driver_data =
> dsi->driver_data;
> +if (dsi->state & flag)
> +return 0;
> +
> samsung_dsim_reset(dsi);
> -samsung_dsim_enable_irq(dsi);
> +
> +if (!(dsi->state & DSIM_STATE_INITIALIZED))
> +samsung_dsim_enable_irq(dsi);
> if (driver_data->reg_values[RESET_TYPE] ==
> DSIM_FUNCRST)
> samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
> @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
> samsung_dsim *dsi)
> samsung_dsim_set_phy_ctrl(dsi);
> samsung_dsim_init_link(dsi);
> +dsi->state |= flag;
> +
> return 0;
> }
> @@ -1269,6 +1276,10 @@ static void
> samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
> }
> dsi->state |= DSIM_STATE_ENABLED;
> +
> +ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
> +if (ret)
> +return;
> }
> static void samsung_dsim_atomic_enable(struct drm_bridge
> *bridge,
> @@ -1458,12 +1469,9 @@ static ssize_t
> samsung_dsim_host_transfer(struct mipi_dsi_host *host,
> if (!(dsi->state & DSIM_STATE_ENABLED))
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
Hi Dave,
On 05.12.22 16:20, Dave Stevenson wrote:
> Hi Frieder
>
> On Mon, 5 Dec 2022 at 07:30, Frieder Schrempf
> wrote:
>>
>> On 02.12.22 15:55, Dave Stevenson wrote:
>>> Hi Marek
>>>
>>> On Fri, 2 Dec 2022 at 12:21, Marek Vasut wrote:
On 12/2/22 11:52, Marek Szyprowski wrote:
> Hi,
>
> Sorry for delay, I was on a sick leave last 2 weeks.
>
> On 28.11.2022 15:43, Jagan Teki wrote:
>> ,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut wrote:
>>> On 11/23/22 21:09, Jagan Teki wrote:
On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut wrote:
> On 11/17/22 14:04, Marek Szyprowski wrote:
>> On 17.11.2022 05:58, Marek Vasut wrote:
>>> On 11/10/22 19:38, Jagan Teki wrote:
DSI host initialization handling in previous exynos dsi driver has
some pitfalls. It initializes the host during host transfer() hook
that is indeed not the desired call flow for I2C and any other DSI
configured downstream bridges.
Host transfer() is usually triggered for downstream DSI panels or
bridges and I2C-configured-DSI bridges miss these host
initialization
as these downstream bridges use bridge operations hooks like
pre_enable,
and enable in order to initialize or set up the host.
This patch is trying to handle the host init handler to satisfy all
downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED
state
flag to ensure that host init is also done on first cmd transfer,
this
helps existing DSI panels work on exynos platform (form Marek
Szyprowski).
v8, v7, v6, v5:
* none
v4:
* update init handling to ensure host init done on first cmd
transfer
v3:
* none
v2:
* check initialized state in samsung_dsim_init
v1:
* keep DSI init in host transfer
Signed-off-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 25
+
include/drm/bridge/samsung-dsim.h | 5 +++--
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index bb1f45fd5a88..ec7e01ae02ea 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
samsung_dsim *dsi)
disable_irq(dsi->irq);
}
-static int samsung_dsim_init(struct samsung_dsim *dsi)
+static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned
int
flag)
{
const struct samsung_dsim_driver_data *driver_data =
dsi->driver_data;
+if (dsi->state & flag)
+return 0;
+
samsung_dsim_reset(dsi);
-samsung_dsim_enable_irq(dsi);
+
+if (!(dsi->state & DSIM_STATE_INITIALIZED))
+samsung_dsim_enable_irq(dsi);
if (driver_data->reg_values[RESET_TYPE] ==
DSIM_FUNCRST)
samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
@@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
samsung_dsim *dsi)
samsung_dsim_set_phy_ctrl(dsi);
samsung_dsim_init_link(dsi);
+dsi->state |= flag;
+
return 0;
}
@@ -1269,6 +1276,10 @@ static void
samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
}
dsi->state |= DSIM_STATE_ENABLED;
+
+ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
+if (ret)
+return;
}
static void samsung_dsim_atomic_enable(struct drm_bridge
*bridge,
@@ -1458,12 +1469,9 @@ static ssize_t
samsung_dsim_host_transfer(struct mipi_dsi_host *host,
if (!(dsi->state & DSIM_STATE_ENABLED))
return -EINVAL;
-if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
-ret = samsung_dsim_init(dsi);
-if (ret)
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
Hi Frieder
On Mon, 5 Dec 2022 at 07:30, Frieder Schrempf
wrote:
>
> On 02.12.22 15:55, Dave Stevenson wrote:
> > Hi Marek
> >
> > On Fri, 2 Dec 2022 at 12:21, Marek Vasut wrote:
> >>
> >> On 12/2/22 11:52, Marek Szyprowski wrote:
> >>> Hi,
> >>>
> >>> Sorry for delay, I was on a sick leave last 2 weeks.
> >>>
> >>> On 28.11.2022 15:43, Jagan Teki wrote:
> ,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut wrote:
> > On 11/23/22 21:09, Jagan Teki wrote:
> >> On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut wrote:
> >>> On 11/17/22 14:04, Marek Szyprowski wrote:
> On 17.11.2022 05:58, Marek Vasut wrote:
> > On 11/10/22 19:38, Jagan Teki wrote:
> >> DSI host initialization handling in previous exynos dsi driver has
> >> some pitfalls. It initializes the host during host transfer() hook
> >> that is indeed not the desired call flow for I2C and any other DSI
> >> configured downstream bridges.
> >>
> >> Host transfer() is usually triggered for downstream DSI panels or
> >> bridges and I2C-configured-DSI bridges miss these host
> >> initialization
> >> as these downstream bridges use bridge operations hooks like
> >> pre_enable,
> >> and enable in order to initialize or set up the host.
> >>
> >> This patch is trying to handle the host init handler to satisfy all
> >> downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED
> >> state
> >> flag to ensure that host init is also done on first cmd transfer,
> >> this
> >> helps existing DSI panels work on exynos platform (form Marek
> >> Szyprowski).
> >>
> >> v8, v7, v6, v5:
> >> * none
> >>
> >> v4:
> >> * update init handling to ensure host init done on first cmd
> >> transfer
> >>
> >> v3:
> >> * none
> >>
> >> v2:
> >> * check initialized state in samsung_dsim_init
> >>
> >> v1:
> >> * keep DSI init in host transfer
> >>
> >> Signed-off-by: Marek Szyprowski
> >> Signed-off-by: Jagan Teki
> >> ---
> >> drivers/gpu/drm/bridge/samsung-dsim.c | 25
> >> +
> >> include/drm/bridge/samsung-dsim.h | 5 +++--
> >> 2 files changed, 20 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> >> b/drivers/gpu/drm/bridge/samsung-dsim.c
> >> index bb1f45fd5a88..ec7e01ae02ea 100644
> >> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> >> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> >> @@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
> >> samsung_dsim *dsi)
> >> disable_irq(dsi->irq);
> >> }
> >> -static int samsung_dsim_init(struct samsung_dsim *dsi)
> >> +static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned
> >> int
> >> flag)
> >> {
> >> const struct samsung_dsim_driver_data *driver_data =
> >> dsi->driver_data;
> >> +if (dsi->state & flag)
> >> +return 0;
> >> +
> >> samsung_dsim_reset(dsi);
> >> -samsung_dsim_enable_irq(dsi);
> >> +
> >> +if (!(dsi->state & DSIM_STATE_INITIALIZED))
> >> +samsung_dsim_enable_irq(dsi);
> >> if (driver_data->reg_values[RESET_TYPE] ==
> >> DSIM_FUNCRST)
> >> samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
> >> @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
> >> samsung_dsim *dsi)
> >> samsung_dsim_set_phy_ctrl(dsi);
> >> samsung_dsim_init_link(dsi);
> >> +dsi->state |= flag;
> >> +
> >> return 0;
> >> }
> >> @@ -1269,6 +1276,10 @@ static void
> >> samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
> >> }
> >> dsi->state |= DSIM_STATE_ENABLED;
> >> +
> >> +ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
> >> +if (ret)
> >> +return;
> >> }
> >> static void samsung_dsim_atomic_enable(struct drm_bridge
> >> *bridge,
> >> @@ -1458,12 +1469,9 @@ static ssize_t
> >> samsung_dsim_host_transfer(struct mipi_dsi_host *host,
> >> if (!(dsi->state & DSIM_STATE_ENABLED))
> >> return -EINVAL;
> >> -if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
> >> -ret = samsung_dsim_init(dsi);
> >> -if (ret)
> >> -return ret;
> >> -
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
On 02.12.22 15:55, Dave Stevenson wrote:
> Hi Marek
>
> On Fri, 2 Dec 2022 at 12:21, Marek Vasut wrote:
>>
>> On 12/2/22 11:52, Marek Szyprowski wrote:
>>> Hi,
>>>
>>> Sorry for delay, I was on a sick leave last 2 weeks.
>>>
>>> On 28.11.2022 15:43, Jagan Teki wrote:
,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut wrote:
> On 11/23/22 21:09, Jagan Teki wrote:
>> On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut wrote:
>>> On 11/17/22 14:04, Marek Szyprowski wrote:
On 17.11.2022 05:58, Marek Vasut wrote:
> On 11/10/22 19:38, Jagan Teki wrote:
>> DSI host initialization handling in previous exynos dsi driver has
>> some pitfalls. It initializes the host during host transfer() hook
>> that is indeed not the desired call flow for I2C and any other DSI
>> configured downstream bridges.
>>
>> Host transfer() is usually triggered for downstream DSI panels or
>> bridges and I2C-configured-DSI bridges miss these host initialization
>> as these downstream bridges use bridge operations hooks like
>> pre_enable,
>> and enable in order to initialize or set up the host.
>>
>> This patch is trying to handle the host init handler to satisfy all
>> downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED
>> state
>> flag to ensure that host init is also done on first cmd transfer,
>> this
>> helps existing DSI panels work on exynos platform (form Marek
>> Szyprowski).
>>
>> v8, v7, v6, v5:
>> * none
>>
>> v4:
>> * update init handling to ensure host init done on first cmd transfer
>>
>> v3:
>> * none
>>
>> v2:
>> * check initialized state in samsung_dsim_init
>>
>> v1:
>> * keep DSI init in host transfer
>>
>> Signed-off-by: Marek Szyprowski
>> Signed-off-by: Jagan Teki
>> ---
>> drivers/gpu/drm/bridge/samsung-dsim.c | 25
>> +
>> include/drm/bridge/samsung-dsim.h | 5 +++--
>> 2 files changed, 20 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>> index bb1f45fd5a88..ec7e01ae02ea 100644
>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>> @@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
>> samsung_dsim *dsi)
>> disable_irq(dsi->irq);
>> }
>> -static int samsung_dsim_init(struct samsung_dsim *dsi)
>> +static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int
>> flag)
>> {
>> const struct samsung_dsim_driver_data *driver_data =
>> dsi->driver_data;
>> +if (dsi->state & flag)
>> +return 0;
>> +
>> samsung_dsim_reset(dsi);
>> -samsung_dsim_enable_irq(dsi);
>> +
>> +if (!(dsi->state & DSIM_STATE_INITIALIZED))
>> +samsung_dsim_enable_irq(dsi);
>> if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
>> samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
>> @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
>> samsung_dsim *dsi)
>> samsung_dsim_set_phy_ctrl(dsi);
>> samsung_dsim_init_link(dsi);
>> +dsi->state |= flag;
>> +
>> return 0;
>> }
>> @@ -1269,6 +1276,10 @@ static void
>> samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
>> }
>> dsi->state |= DSIM_STATE_ENABLED;
>> +
>> +ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
>> +if (ret)
>> +return;
>> }
>> static void samsung_dsim_atomic_enable(struct drm_bridge
>> *bridge,
>> @@ -1458,12 +1469,9 @@ static ssize_t
>> samsung_dsim_host_transfer(struct mipi_dsi_host *host,
>> if (!(dsi->state & DSIM_STATE_ENABLED))
>> return -EINVAL;
>> -if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
>> -ret = samsung_dsim_init(dsi);
>> -if (ret)
>> -return ret;
>> -dsi->state |= DSIM_STATE_INITIALIZED;
>> -}
>> +ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
> This triggers full controller reset and reprogramming upon first
> command transfer, is such heavy handed reload really necessary ?
Yes it is, otherwise the proper DSI panels doesn't work with
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
Hi Marek
On Fri, 2 Dec 2022 at 12:21, Marek Vasut wrote:
>
> On 12/2/22 11:52, Marek Szyprowski wrote:
> > Hi,
> >
> > Sorry for delay, I was on a sick leave last 2 weeks.
> >
> > On 28.11.2022 15:43, Jagan Teki wrote:
> >> ,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut wrote:
> >>> On 11/23/22 21:09, Jagan Teki wrote:
> On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut wrote:
> > On 11/17/22 14:04, Marek Szyprowski wrote:
> >> On 17.11.2022 05:58, Marek Vasut wrote:
> >>> On 11/10/22 19:38, Jagan Teki wrote:
> DSI host initialization handling in previous exynos dsi driver has
> some pitfalls. It initializes the host during host transfer() hook
> that is indeed not the desired call flow for I2C and any other DSI
> configured downstream bridges.
>
> Host transfer() is usually triggered for downstream DSI panels or
> bridges and I2C-configured-DSI bridges miss these host initialization
> as these downstream bridges use bridge operations hooks like
> pre_enable,
> and enable in order to initialize or set up the host.
>
> This patch is trying to handle the host init handler to satisfy all
> downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED
> state
> flag to ensure that host init is also done on first cmd transfer,
> this
> helps existing DSI panels work on exynos platform (form Marek
> Szyprowski).
>
> v8, v7, v6, v5:
> * none
>
> v4:
> * update init handling to ensure host init done on first cmd transfer
>
> v3:
> * none
>
> v2:
> * check initialized state in samsung_dsim_init
>
> v1:
> * keep DSI init in host transfer
>
> Signed-off-by: Marek Szyprowski
> Signed-off-by: Jagan Teki
> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 25
> +
> include/drm/bridge/samsung-dsim.h | 5 +++--
> 2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> b/drivers/gpu/drm/bridge/samsung-dsim.c
> index bb1f45fd5a88..ec7e01ae02ea 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
> samsung_dsim *dsi)
> disable_irq(dsi->irq);
> }
> -static int samsung_dsim_init(struct samsung_dsim *dsi)
> +static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int
> flag)
> {
> const struct samsung_dsim_driver_data *driver_data =
> dsi->driver_data;
> +if (dsi->state & flag)
> +return 0;
> +
> samsung_dsim_reset(dsi);
> -samsung_dsim_enable_irq(dsi);
> +
> +if (!(dsi->state & DSIM_STATE_INITIALIZED))
> +samsung_dsim_enable_irq(dsi);
> if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
> samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
> @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
> samsung_dsim *dsi)
> samsung_dsim_set_phy_ctrl(dsi);
> samsung_dsim_init_link(dsi);
> +dsi->state |= flag;
> +
> return 0;
> }
> @@ -1269,6 +1276,10 @@ static void
> samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
> }
> dsi->state |= DSIM_STATE_ENABLED;
> +
> +ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
> +if (ret)
> +return;
> }
> static void samsung_dsim_atomic_enable(struct drm_bridge
> *bridge,
> @@ -1458,12 +1469,9 @@ static ssize_t
> samsung_dsim_host_transfer(struct mipi_dsi_host *host,
> if (!(dsi->state & DSIM_STATE_ENABLED))
> return -EINVAL;
> -if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
> -ret = samsung_dsim_init(dsi);
> -if (ret)
> -return ret;
> -dsi->state |= DSIM_STATE_INITIALIZED;
> -}
> +ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
> >>> This triggers full controller reset and reprogramming upon first
> >>> command transfer, is such heavy handed reload really necessary ?
> >> Yes it is, otherwise the proper DSI panels doesn't work with Exynos DRM
> >> DSI. If this is a real issue
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
On 12/2/22 11:52, Marek Szyprowski wrote:
Hi,
Sorry for delay, I was on a sick leave last 2 weeks.
On 28.11.2022 15:43, Jagan Teki wrote:
,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut wrote:
On 11/23/22 21:09, Jagan Teki wrote:
On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut wrote:
On 11/17/22 14:04, Marek Szyprowski wrote:
On 17.11.2022 05:58, Marek Vasut wrote:
On 11/10/22 19:38, Jagan Teki wrote:
DSI host initialization handling in previous exynos dsi driver has
some pitfalls. It initializes the host during host transfer() hook
that is indeed not the desired call flow for I2C and any other DSI
configured downstream bridges.
Host transfer() is usually triggered for downstream DSI panels or
bridges and I2C-configured-DSI bridges miss these host initialization
as these downstream bridges use bridge operations hooks like pre_enable,
and enable in order to initialize or set up the host.
This patch is trying to handle the host init handler to satisfy all
downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state
flag to ensure that host init is also done on first cmd transfer, this
helps existing DSI panels work on exynos platform (form Marek
Szyprowski).
v8, v7, v6, v5:
* none
v4:
* update init handling to ensure host init done on first cmd transfer
v3:
* none
v2:
* check initialized state in samsung_dsim_init
v1:
* keep DSI init in host transfer
Signed-off-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 25 +
include/drm/bridge/samsung-dsim.h | 5 +++--
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index bb1f45fd5a88..ec7e01ae02ea 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
samsung_dsim *dsi)
disable_irq(dsi->irq);
}
-static int samsung_dsim_init(struct samsung_dsim *dsi)
+static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int
flag)
{
const struct samsung_dsim_driver_data *driver_data =
dsi->driver_data;
+if (dsi->state & flag)
+return 0;
+
samsung_dsim_reset(dsi);
-samsung_dsim_enable_irq(dsi);
+
+if (!(dsi->state & DSIM_STATE_INITIALIZED))
+samsung_dsim_enable_irq(dsi);
if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
@@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
samsung_dsim *dsi)
samsung_dsim_set_phy_ctrl(dsi);
samsung_dsim_init_link(dsi);
+dsi->state |= flag;
+
return 0;
}
@@ -1269,6 +1276,10 @@ static void
samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
}
dsi->state |= DSIM_STATE_ENABLED;
+
+ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
+if (ret)
+return;
}
static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
@@ -1458,12 +1469,9 @@ static ssize_t
samsung_dsim_host_transfer(struct mipi_dsi_host *host,
if (!(dsi->state & DSIM_STATE_ENABLED))
return -EINVAL;
-if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
-ret = samsung_dsim_init(dsi);
-if (ret)
-return ret;
-dsi->state |= DSIM_STATE_INITIALIZED;
-}
+ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
This triggers full controller reset and reprogramming upon first
command transfer, is such heavy handed reload really necessary ?
Yes it is, otherwise the proper DSI panels doesn't work with Exynos DRM
DSI. If this is a real issue for you, then maybe the driver could do the
initialization conditionally, in prepare() callback in case of IMX and
on the first transfer in case of Exynos?
That's odd , it does actually break panel support for me, without this
double reset the panel works again. But I have to wonder, why would such
a full reset be necessary at all , even on the exynos ?
Is it breaking samsung_dsim_reset from host_transfer? maybe checking
whether a reset is required before calling it might fix the issue. I
agree with double initialization is odd but it seems it is required on
some panels in Exynos, I think tweaking them and adjusting the panel
code might resolve this discrepancy.
Can someone provide further details on the exynos problem ?
If I'm correct this sequence is required in order to work the existing
panel/bridges on exynos. Adjusting these panel/bridge codes can
possibly fix the sequence further.
Marek Szyprowski, please add if you have anything.
Well, frankly speaking the double initialization is not a correct
sequence, but this is the only one that actually works on Exynos based
boards with DSI panels after moving the initialization to bridge's
.prepare() callback.
Somehow, I suspect this is
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
Hi,
Sorry for delay, I was on a sick leave last 2 weeks.
On 28.11.2022 15:43, Jagan Teki wrote:
> ,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut wrote:
>> On 11/23/22 21:09, Jagan Teki wrote:
>>> On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut wrote:
On 11/17/22 14:04, Marek Szyprowski wrote:
> On 17.11.2022 05:58, Marek Vasut wrote:
>> On 11/10/22 19:38, Jagan Teki wrote:
>>> DSI host initialization handling in previous exynos dsi driver has
>>> some pitfalls. It initializes the host during host transfer() hook
>>> that is indeed not the desired call flow for I2C and any other DSI
>>> configured downstream bridges.
>>>
>>> Host transfer() is usually triggered for downstream DSI panels or
>>> bridges and I2C-configured-DSI bridges miss these host initialization
>>> as these downstream bridges use bridge operations hooks like pre_enable,
>>> and enable in order to initialize or set up the host.
>>>
>>> This patch is trying to handle the host init handler to satisfy all
>>> downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state
>>> flag to ensure that host init is also done on first cmd transfer, this
>>> helps existing DSI panels work on exynos platform (form Marek
>>> Szyprowski).
>>>
>>> v8, v7, v6, v5:
>>> * none
>>>
>>> v4:
>>> * update init handling to ensure host init done on first cmd transfer
>>>
>>> v3:
>>> * none
>>>
>>> v2:
>>> * check initialized state in samsung_dsim_init
>>>
>>> v1:
>>> * keep DSI init in host transfer
>>>
>>> Signed-off-by: Marek Szyprowski
>>> Signed-off-by: Jagan Teki
>>> ---
>>> drivers/gpu/drm/bridge/samsung-dsim.c | 25
>>> +
>>> include/drm/bridge/samsung-dsim.h | 5 +++--
>>> 2 files changed, 20 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
>>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> index bb1f45fd5a88..ec7e01ae02ea 100644
>>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> @@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
>>> samsung_dsim *dsi)
>>> disable_irq(dsi->irq);
>>> }
>>> -static int samsung_dsim_init(struct samsung_dsim *dsi)
>>> +static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int
>>> flag)
>>> {
>>> const struct samsung_dsim_driver_data *driver_data =
>>> dsi->driver_data;
>>> +if (dsi->state & flag)
>>> +return 0;
>>> +
>>> samsung_dsim_reset(dsi);
>>> -samsung_dsim_enable_irq(dsi);
>>> +
>>> +if (!(dsi->state & DSIM_STATE_INITIALIZED))
>>> +samsung_dsim_enable_irq(dsi);
>>>if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
>>> samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
>>> @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
>>> samsung_dsim *dsi)
>>> samsung_dsim_set_phy_ctrl(dsi);
>>> samsung_dsim_init_link(dsi);
>>> +dsi->state |= flag;
>>> +
>>> return 0;
>>> }
>>> @@ -1269,6 +1276,10 @@ static void
>>> samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
>>> }
>>>dsi->state |= DSIM_STATE_ENABLED;
>>> +
>>> +ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
>>> +if (ret)
>>> +return;
>>> }
>>>static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
>>> @@ -1458,12 +1469,9 @@ static ssize_t
>>> samsung_dsim_host_transfer(struct mipi_dsi_host *host,
>>> if (!(dsi->state & DSIM_STATE_ENABLED))
>>> return -EINVAL;
>>> -if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
>>> -ret = samsung_dsim_init(dsi);
>>> -if (ret)
>>> -return ret;
>>> -dsi->state |= DSIM_STATE_INITIALIZED;
>>> -}
>>> +ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
>> This triggers full controller reset and reprogramming upon first
>> command transfer, is such heavy handed reload really necessary ?
> Yes it is, otherwise the proper DSI panels doesn't work with Exynos DRM
> DSI. If this is a real issue for you, then maybe the driver could do the
> initialization conditionally, in prepare() callback in case of IMX and
> on the first transfer in case of Exynos?
That's odd , it does actually break panel support for me, without this
double reset the panel works again. But I have to wonder, why would such
a full reset be necessary at all , even on the exynos ?
>>> Is it breaking samsung_dsim_reset from host_transfer? maybe checking
>>> whether a reset is required before calling it
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut wrote:
>
> On 11/23/22 21:09, Jagan Teki wrote:
> > On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut wrote:
> >>
> >> On 11/17/22 14:04, Marek Szyprowski wrote:
> >>> On 17.11.2022 05:58, Marek Vasut wrote:
> On 11/10/22 19:38, Jagan Teki wrote:
> > DSI host initialization handling in previous exynos dsi driver has
> > some pitfalls. It initializes the host during host transfer() hook
> > that is indeed not the desired call flow for I2C and any other DSI
> > configured downstream bridges.
> >
> > Host transfer() is usually triggered for downstream DSI panels or
> > bridges and I2C-configured-DSI bridges miss these host initialization
> > as these downstream bridges use bridge operations hooks like pre_enable,
> > and enable in order to initialize or set up the host.
> >
> > This patch is trying to handle the host init handler to satisfy all
> > downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state
> > flag to ensure that host init is also done on first cmd transfer, this
> > helps existing DSI panels work on exynos platform (form Marek
> > Szyprowski).
> >
> > v8, v7, v6, v5:
> > * none
> >
> > v4:
> > * update init handling to ensure host init done on first cmd transfer
> >
> > v3:
> > * none
> >
> > v2:
> > * check initialized state in samsung_dsim_init
> >
> > v1:
> > * keep DSI init in host transfer
> >
> > Signed-off-by: Marek Szyprowski
> > Signed-off-by: Jagan Teki
> > ---
> > drivers/gpu/drm/bridge/samsung-dsim.c | 25 +
> > include/drm/bridge/samsung-dsim.h | 5 +++--
> > 2 files changed, 20 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> > b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index bb1f45fd5a88..ec7e01ae02ea 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
> > samsung_dsim *dsi)
> > disable_irq(dsi->irq);
> > }
> > -static int samsung_dsim_init(struct samsung_dsim *dsi)
> > +static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int
> > flag)
> > {
> > const struct samsung_dsim_driver_data *driver_data =
> > dsi->driver_data;
> > +if (dsi->state & flag)
> > +return 0;
> > +
> > samsung_dsim_reset(dsi);
> > -samsung_dsim_enable_irq(dsi);
> > +
> > +if (!(dsi->state & DSIM_STATE_INITIALIZED))
> > +samsung_dsim_enable_irq(dsi);
> > if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
> > samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
> > @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
> > samsung_dsim *dsi)
> > samsung_dsim_set_phy_ctrl(dsi);
> > samsung_dsim_init_link(dsi);
> > +dsi->state |= flag;
> > +
> > return 0;
> > }
> > @@ -1269,6 +1276,10 @@ static void
> > samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
> > }
> > dsi->state |= DSIM_STATE_ENABLED;
> > +
> > +ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
> > +if (ret)
> > +return;
> > }
> > static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
> > @@ -1458,12 +1469,9 @@ static ssize_t
> > samsung_dsim_host_transfer(struct mipi_dsi_host *host,
> > if (!(dsi->state & DSIM_STATE_ENABLED))
> > return -EINVAL;
> > -if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
> > -ret = samsung_dsim_init(dsi);
> > -if (ret)
> > -return ret;
> > -dsi->state |= DSIM_STATE_INITIALIZED;
> > -}
> > +ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
>
> This triggers full controller reset and reprogramming upon first
> command transfer, is such heavy handed reload really necessary ?
> >>>
> >>> Yes it is, otherwise the proper DSI panels doesn't work with Exynos DRM
> >>> DSI. If this is a real issue for you, then maybe the driver could do the
> >>> initialization conditionally, in prepare() callback in case of IMX and
> >>> on the first transfer in case of Exynos?
> >>
> >> That's odd , it does actually break panel support for me, without this
> >> double reset the panel works again. But I have to wonder, why would such
> >> a full reset be necessary at all , even on the exynos ?
> >
> > Is it breaking samsung_dsim_reset from host_transfer? maybe checking
> > whether a reset is required before calling it might fix the issue. I
> > agree with double initialization is odd but it seems it is required on
> >
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
On 11/23/22 21:09, Jagan Teki wrote:
On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut wrote:
On 11/17/22 14:04, Marek Szyprowski wrote:
On 17.11.2022 05:58, Marek Vasut wrote:
On 11/10/22 19:38, Jagan Teki wrote:
DSI host initialization handling in previous exynos dsi driver has
some pitfalls. It initializes the host during host transfer() hook
that is indeed not the desired call flow for I2C and any other DSI
configured downstream bridges.
Host transfer() is usually triggered for downstream DSI panels or
bridges and I2C-configured-DSI bridges miss these host initialization
as these downstream bridges use bridge operations hooks like pre_enable,
and enable in order to initialize or set up the host.
This patch is trying to handle the host init handler to satisfy all
downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state
flag to ensure that host init is also done on first cmd transfer, this
helps existing DSI panels work on exynos platform (form Marek
Szyprowski).
v8, v7, v6, v5:
* none
v4:
* update init handling to ensure host init done on first cmd transfer
v3:
* none
v2:
* check initialized state in samsung_dsim_init
v1:
* keep DSI init in host transfer
Signed-off-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 25 +
include/drm/bridge/samsung-dsim.h | 5 +++--
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index bb1f45fd5a88..ec7e01ae02ea 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
samsung_dsim *dsi)
disable_irq(dsi->irq);
}
-static int samsung_dsim_init(struct samsung_dsim *dsi)
+static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int
flag)
{
const struct samsung_dsim_driver_data *driver_data =
dsi->driver_data;
+if (dsi->state & flag)
+return 0;
+
samsung_dsim_reset(dsi);
-samsung_dsim_enable_irq(dsi);
+
+if (!(dsi->state & DSIM_STATE_INITIALIZED))
+samsung_dsim_enable_irq(dsi);
if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
@@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
samsung_dsim *dsi)
samsung_dsim_set_phy_ctrl(dsi);
samsung_dsim_init_link(dsi);
+dsi->state |= flag;
+
return 0;
}
@@ -1269,6 +1276,10 @@ static void
samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
}
dsi->state |= DSIM_STATE_ENABLED;
+
+ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
+if (ret)
+return;
}
static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
@@ -1458,12 +1469,9 @@ static ssize_t
samsung_dsim_host_transfer(struct mipi_dsi_host *host,
if (!(dsi->state & DSIM_STATE_ENABLED))
return -EINVAL;
-if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
-ret = samsung_dsim_init(dsi);
-if (ret)
-return ret;
-dsi->state |= DSIM_STATE_INITIALIZED;
-}
+ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
This triggers full controller reset and reprogramming upon first
command transfer, is such heavy handed reload really necessary ?
Yes it is, otherwise the proper DSI panels doesn't work with Exynos DRM
DSI. If this is a real issue for you, then maybe the driver could do the
initialization conditionally, in prepare() callback in case of IMX and
on the first transfer in case of Exynos?
That's odd , it does actually break panel support for me, without this
double reset the panel works again. But I have to wonder, why would such
a full reset be necessary at all , even on the exynos ?
Is it breaking samsung_dsim_reset from host_transfer? maybe checking
whether a reset is required before calling it might fix the issue. I
agree with double initialization is odd but it seems it is required on
some panels in Exynos, I think tweaking them and adjusting the panel
code might resolve this discrepancy.
Can someone provide further details on the exynos problem ?
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut wrote:
>
> On 11/17/22 14:04, Marek Szyprowski wrote:
> > On 17.11.2022 05:58, Marek Vasut wrote:
> >> On 11/10/22 19:38, Jagan Teki wrote:
> >>> DSI host initialization handling in previous exynos dsi driver has
> >>> some pitfalls. It initializes the host during host transfer() hook
> >>> that is indeed not the desired call flow for I2C and any other DSI
> >>> configured downstream bridges.
> >>>
> >>> Host transfer() is usually triggered for downstream DSI panels or
> >>> bridges and I2C-configured-DSI bridges miss these host initialization
> >>> as these downstream bridges use bridge operations hooks like pre_enable,
> >>> and enable in order to initialize or set up the host.
> >>>
> >>> This patch is trying to handle the host init handler to satisfy all
> >>> downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state
> >>> flag to ensure that host init is also done on first cmd transfer, this
> >>> helps existing DSI panels work on exynos platform (form Marek
> >>> Szyprowski).
> >>>
> >>> v8, v7, v6, v5:
> >>> * none
> >>>
> >>> v4:
> >>> * update init handling to ensure host init done on first cmd transfer
> >>>
> >>> v3:
> >>> * none
> >>>
> >>> v2:
> >>> * check initialized state in samsung_dsim_init
> >>>
> >>> v1:
> >>> * keep DSI init in host transfer
> >>>
> >>> Signed-off-by: Marek Szyprowski
> >>> Signed-off-by: Jagan Teki
> >>> ---
> >>>drivers/gpu/drm/bridge/samsung-dsim.c | 25 +
> >>>include/drm/bridge/samsung-dsim.h | 5 +++--
> >>>2 files changed, 20 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> >>> b/drivers/gpu/drm/bridge/samsung-dsim.c
> >>> index bb1f45fd5a88..ec7e01ae02ea 100644
> >>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> >>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> >>> @@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
> >>> samsung_dsim *dsi)
> >>>disable_irq(dsi->irq);
> >>>}
> >>>-static int samsung_dsim_init(struct samsung_dsim *dsi)
> >>> +static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int
> >>> flag)
> >>>{
> >>>const struct samsung_dsim_driver_data *driver_data =
> >>> dsi->driver_data;
> >>>+if (dsi->state & flag)
> >>> +return 0;
> >>> +
> >>>samsung_dsim_reset(dsi);
> >>> -samsung_dsim_enable_irq(dsi);
> >>> +
> >>> +if (!(dsi->state & DSIM_STATE_INITIALIZED))
> >>> +samsung_dsim_enable_irq(dsi);
> >>> if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
> >>>samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
> >>> @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
> >>> samsung_dsim *dsi)
> >>>samsung_dsim_set_phy_ctrl(dsi);
> >>>samsung_dsim_init_link(dsi);
> >>>+dsi->state |= flag;
> >>> +
> >>>return 0;
> >>>}
> >>>@@ -1269,6 +1276,10 @@ static void
> >>> samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
> >>>}
> >>> dsi->state |= DSIM_STATE_ENABLED;
> >>> +
> >>> +ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
> >>> +if (ret)
> >>> +return;
> >>>}
> >>> static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
> >>> @@ -1458,12 +1469,9 @@ static ssize_t
> >>> samsung_dsim_host_transfer(struct mipi_dsi_host *host,
> >>>if (!(dsi->state & DSIM_STATE_ENABLED))
> >>>return -EINVAL;
> >>>-if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
> >>> -ret = samsung_dsim_init(dsi);
> >>> -if (ret)
> >>> -return ret;
> >>> -dsi->state |= DSIM_STATE_INITIALIZED;
> >>> -}
> >>> +ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
> >>
> >> This triggers full controller reset and reprogramming upon first
> >> command transfer, is such heavy handed reload really necessary ?
> >
> > Yes it is, otherwise the proper DSI panels doesn't work with Exynos DRM
> > DSI. If this is a real issue for you, then maybe the driver could do the
> > initialization conditionally, in prepare() callback in case of IMX and
> > on the first transfer in case of Exynos?
>
> That's odd , it does actually break panel support for me, without this
> double reset the panel works again. But I have to wonder, why would such
> a full reset be necessary at all , even on the exynos ?
Is it breaking samsung_dsim_reset from host_transfer? maybe checking
whether a reset is required before calling it might fix the issue. I
agree with double initialization is odd but it seems it is required on
some panels in Exynos, I think tweaking them and adjusting the panel
code might resolve this discrepancy.
Jagan.
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
On 11/17/22 14:04, Marek Szyprowski wrote:
On 17.11.2022 05:58, Marek Vasut wrote:
On 11/10/22 19:38, Jagan Teki wrote:
DSI host initialization handling in previous exynos dsi driver has
some pitfalls. It initializes the host during host transfer() hook
that is indeed not the desired call flow for I2C and any other DSI
configured downstream bridges.
Host transfer() is usually triggered for downstream DSI panels or
bridges and I2C-configured-DSI bridges miss these host initialization
as these downstream bridges use bridge operations hooks like pre_enable,
and enable in order to initialize or set up the host.
This patch is trying to handle the host init handler to satisfy all
downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state
flag to ensure that host init is also done on first cmd transfer, this
helps existing DSI panels work on exynos platform (form Marek
Szyprowski).
v8, v7, v6, v5:
* none
v4:
* update init handling to ensure host init done on first cmd transfer
v3:
* none
v2:
* check initialized state in samsung_dsim_init
v1:
* keep DSI init in host transfer
Signed-off-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 25 +
include/drm/bridge/samsung-dsim.h | 5 +++--
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index bb1f45fd5a88..ec7e01ae02ea 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
samsung_dsim *dsi)
disable_irq(dsi->irq);
}
-static int samsung_dsim_init(struct samsung_dsim *dsi)
+static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int
flag)
{
const struct samsung_dsim_driver_data *driver_data =
dsi->driver_data;
+ if (dsi->state & flag)
+ return 0;
+
samsung_dsim_reset(dsi);
- samsung_dsim_enable_irq(dsi);
+
+ if (!(dsi->state & DSIM_STATE_INITIALIZED))
+ samsung_dsim_enable_irq(dsi);
if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
@@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
samsung_dsim *dsi)
samsung_dsim_set_phy_ctrl(dsi);
samsung_dsim_init_link(dsi);
+ dsi->state |= flag;
+
return 0;
}
@@ -1269,6 +1276,10 @@ static void
samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
}
dsi->state |= DSIM_STATE_ENABLED;
+
+ ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
+ if (ret)
+ return;
}
static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
@@ -1458,12 +1469,9 @@ static ssize_t
samsung_dsim_host_transfer(struct mipi_dsi_host *host,
if (!(dsi->state & DSIM_STATE_ENABLED))
return -EINVAL;
- if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
- ret = samsung_dsim_init(dsi);
- if (ret)
- return ret;
- dsi->state |= DSIM_STATE_INITIALIZED;
- }
+ ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
This triggers full controller reset and reprogramming upon first
command transfer, is such heavy handed reload really necessary ?
Yes it is, otherwise the proper DSI panels doesn't work with Exynos DRM
DSI. If this is a real issue for you, then maybe the driver could do the
initialization conditionally, in prepare() callback in case of IMX and
on the first transfer in case of Exynos?
That's odd , it does actually break panel support for me, without this
double reset the panel works again. But I have to wonder, why would such
a full reset be necessary at all , even on the exynos ?
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
On 17.11.2022 05:58, Marek Vasut wrote:
> On 11/10/22 19:38, Jagan Teki wrote:
>> DSI host initialization handling in previous exynos dsi driver has
>> some pitfalls. It initializes the host during host transfer() hook
>> that is indeed not the desired call flow for I2C and any other DSI
>> configured downstream bridges.
>>
>> Host transfer() is usually triggered for downstream DSI panels or
>> bridges and I2C-configured-DSI bridges miss these host initialization
>> as these downstream bridges use bridge operations hooks like pre_enable,
>> and enable in order to initialize or set up the host.
>>
>> This patch is trying to handle the host init handler to satisfy all
>> downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state
>> flag to ensure that host init is also done on first cmd transfer, this
>> helps existing DSI panels work on exynos platform (form Marek
>> Szyprowski).
>>
>> v8, v7, v6, v5:
>> * none
>>
>> v4:
>> * update init handling to ensure host init done on first cmd transfer
>>
>> v3:
>> * none
>>
>> v2:
>> * check initialized state in samsung_dsim_init
>>
>> v1:
>> * keep DSI init in host transfer
>>
>> Signed-off-by: Marek Szyprowski
>> Signed-off-by: Jagan Teki
>> ---
>> drivers/gpu/drm/bridge/samsung-dsim.c | 25 +
>> include/drm/bridge/samsung-dsim.h | 5 +++--
>> 2 files changed, 20 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>> index bb1f45fd5a88..ec7e01ae02ea 100644
>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>> @@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
>> samsung_dsim *dsi)
>> disable_irq(dsi->irq);
>> }
>> -static int samsung_dsim_init(struct samsung_dsim *dsi)
>> +static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int
>> flag)
>> {
>> const struct samsung_dsim_driver_data *driver_data =
>> dsi->driver_data;
>> + if (dsi->state & flag)
>> + return 0;
>> +
>> samsung_dsim_reset(dsi);
>> - samsung_dsim_enable_irq(dsi);
>> +
>> + if (!(dsi->state & DSIM_STATE_INITIALIZED))
>> + samsung_dsim_enable_irq(dsi);
>> if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
>> samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
>> @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
>> samsung_dsim *dsi)
>> samsung_dsim_set_phy_ctrl(dsi);
>> samsung_dsim_init_link(dsi);
>> + dsi->state |= flag;
>> +
>> return 0;
>> }
>> @@ -1269,6 +1276,10 @@ static void
>> samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
>> }
>> dsi->state |= DSIM_STATE_ENABLED;
>> +
>> + ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
>> + if (ret)
>> + return;
>> }
>> static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
>> @@ -1458,12 +1469,9 @@ static ssize_t
>> samsung_dsim_host_transfer(struct mipi_dsi_host *host,
>> if (!(dsi->state & DSIM_STATE_ENABLED))
>> return -EINVAL;
>> - if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
>> - ret = samsung_dsim_init(dsi);
>> - if (ret)
>> - return ret;
>> - dsi->state |= DSIM_STATE_INITIALIZED;
>> - }
>> + ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
>
> This triggers full controller reset and reprogramming upon first
> command transfer, is such heavy handed reload really necessary ?
Yes it is, otherwise the proper DSI panels doesn't work with Exynos DRM
DSI. If this is a real issue for you, then maybe the driver could do the
initialization conditionally, in prepare() callback in case of IMX and
on the first transfer in case of Exynos?
Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
On 11/10/22 19:38, Jagan Teki wrote:
DSI host initialization handling in previous exynos dsi driver has
some pitfalls. It initializes the host during host transfer() hook
that is indeed not the desired call flow for I2C and any other DSI
configured downstream bridges.
Host transfer() is usually triggered for downstream DSI panels or
bridges and I2C-configured-DSI bridges miss these host initialization
as these downstream bridges use bridge operations hooks like pre_enable,
and enable in order to initialize or set up the host.
This patch is trying to handle the host init handler to satisfy all
downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state
flag to ensure that host init is also done on first cmd transfer, this
helps existing DSI panels work on exynos platform (form Marek
Szyprowski).
v8, v7, v6, v5:
* none
v4:
* update init handling to ensure host init done on first cmd transfer
v3:
* none
v2:
* check initialized state in samsung_dsim_init
v1:
* keep DSI init in host transfer
Signed-off-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 25 +
include/drm/bridge/samsung-dsim.h | 5 +++--
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index bb1f45fd5a88..ec7e01ae02ea 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
samsung_dsim *dsi)
disable_irq(dsi->irq);
}
-static int samsung_dsim_init(struct samsung_dsim *dsi)
+static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int flag)
{
const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
+ if (dsi->state & flag)
+ return 0;
+
samsung_dsim_reset(dsi);
- samsung_dsim_enable_irq(dsi);
+
+ if (!(dsi->state & DSIM_STATE_INITIALIZED))
+ samsung_dsim_enable_irq(dsi);
if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
@@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct samsung_dsim *dsi)
samsung_dsim_set_phy_ctrl(dsi);
samsung_dsim_init_link(dsi);
+ dsi->state |= flag;
+
return 0;
}
@@ -1269,6 +1276,10 @@ static void samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
}
dsi->state |= DSIM_STATE_ENABLED;
+
+ ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
+ if (ret)
+ return;
}
static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
@@ -1458,12 +1469,9 @@ static ssize_t samsung_dsim_host_transfer(struct
mipi_dsi_host *host,
if (!(dsi->state & DSIM_STATE_ENABLED))
return -EINVAL;
- if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
- ret = samsung_dsim_init(dsi);
- if (ret)
- return ret;
- dsi->state |= DSIM_STATE_INITIALIZED;
- }
+ ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
This triggers full controller reset and reprogramming upon first command
transfer, is such heavy handed reload really necessary ?
