Re: [PATCH v2 1/2] drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec

2023-07-20 Thread Frieder Schrempf
On 19.07.23 18:34, Tim Harvey wrote:
> On Wed, Jul 19, 2023 at 12:05 AM Frieder Schrempf
>  wrote:
>>
>> Hi Tim,
>>
>> On 19.07.23 01:03, Tim Harvey wrote:
>>> On Thu, Jul 13, 2023 at 3:01 AM Frieder Schrempf
>>>  wrote:

 Hi Tim,

 On 13.07.23 09:18, Frieder Schrempf wrote:
> Hi Tim,
>
> On 13.07.23 00:34, Tim Harvey wrote:
>> On Wed, May 3, 2023 at 9:33 AM Frieder Schrempf  wrote:
>>>
>>> From: Frieder Schrempf 
>>>
>>> According to the documentation [1] the proper enable flow is:
>>>
>>> 1. Enable DSI link and keep data lanes in LP-11 (stop state)
>>> 2. Disable stop state to bring data lanes into HS mode
>>>
>>> Currently we do this all at once within enable(), which doesn't
>>> allow to meet the requirements of some downstream bridges.
>>>
>>> To fix this we now enable the DSI in pre_enable() and force it
>>> into stop state using the FORCE_STOP_STATE bit in the ESCMODE
>>> register until enable() is called where we reset the bit.
>>>
>>> We currently do this only for i.MX8M as Exynos uses a different
>>> init flow where samsung_dsim_init() is called from
>>> samsung_dsim_host_transfer().
>>>
>>> [1] 
>>> https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
>>>
>>> Signed-off-by: Frieder Schrempf 
>>> ---
>>> Changes for v2:
>>> * Drop RFC
>>> ---
>>>  drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++--
>>>  1 file changed, 23 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
>>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> index e0a402a85787..9775779721d9 100644
>>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> @@ -859,6 +859,10 @@ static int samsung_dsim_init_link(struct 
>>> samsung_dsim *dsi)
>>> reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
>>> reg &= ~DSIM_STOP_STATE_CNT_MASK;
>>> reg |= 
>>> DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
>>> +
>>> +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
>>> +   reg |= DSIM_FORCE_STOP_STATE;
>>> +
>>> samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>>>
>>> reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
>>> @@ -1340,6 +1344,9 @@ static void samsung_dsim_atomic_pre_enable(struct 
>>> drm_bridge *bridge,
>>> ret = samsung_dsim_init(dsi);
>>> if (ret)
>>> return;
>>> +
>>> +   samsung_dsim_set_display_mode(dsi);
>>> +   samsung_dsim_set_display_enable(dsi, true);
>>> }
>>>  }
>>>
>>> @@ -1347,9 +1354,16 @@ static void samsung_dsim_atomic_enable(struct 
>>> drm_bridge *bridge,
>>>struct drm_bridge_state 
>>> *old_bridge_state)
>>>  {
>>> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
>>> +   u32 reg;
>>>
>>> -   samsung_dsim_set_display_mode(dsi);
>>> -   samsung_dsim_set_display_enable(dsi, true);
>>> +   if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
>>> +   samsung_dsim_set_display_mode(dsi);
>>> +   samsung_dsim_set_display_enable(dsi, true);
>>> +   } else {
>>> +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
>>> +   reg &= ~DSIM_FORCE_STOP_STATE;
>>> +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>>> +   }
>>>
>>> dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
>>>  }
>>> @@ -1358,10 +1372,17 @@ static void samsung_dsim_atomic_disable(struct 
>>> drm_bridge *bridge,
>>> struct drm_bridge_state 
>>> *old_bridge_state)
>>>  {
>>> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
>>> +   u32 reg;
>>>
>>> if (!(dsi->state & DSIM_STATE_ENABLED))
>>> return;
>>>
>>> +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
>>> +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
>>> +   reg |= DSIM_FORCE_STOP_STATE;
>>> +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>>> +   }
>>> +
>>> dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
>>>  }
>>>
>>> --
>>> 2.40.0
>>>
>>
>> Hi Frieder,
>>
>> I found this patch to break mipi-dsi display on my board which has:
>>  - FocalTech FT5406 10pt touch controller (with no interrupt)
>>  - Powertip PH800480T013-IDF02 compatible panel
>>  - Toshiba TC358762 compatible DSI to DBI bridge
>>  - ATTINY based regulator used for backlight controller and panel enable

Re: [PATCH v2 1/2] drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec

2023-07-19 Thread Tim Harvey
On Wed, Jul 19, 2023 at 12:05 AM Frieder Schrempf
 wrote:
>
> Hi Tim,
>
> On 19.07.23 01:03, Tim Harvey wrote:
> > On Thu, Jul 13, 2023 at 3:01 AM Frieder Schrempf
> >  wrote:
> >>
> >> Hi Tim,
> >>
> >> On 13.07.23 09:18, Frieder Schrempf wrote:
> >>> Hi Tim,
> >>>
> >>> On 13.07.23 00:34, Tim Harvey wrote:
>  On Wed, May 3, 2023 at 9:33 AM Frieder Schrempf  wrote:
> >
> > From: Frieder Schrempf 
> >
> > According to the documentation [1] the proper enable flow is:
> >
> > 1. Enable DSI link and keep data lanes in LP-11 (stop state)
> > 2. Disable stop state to bring data lanes into HS mode
> >
> > Currently we do this all at once within enable(), which doesn't
> > allow to meet the requirements of some downstream bridges.
> >
> > To fix this we now enable the DSI in pre_enable() and force it
> > into stop state using the FORCE_STOP_STATE bit in the ESCMODE
> > register until enable() is called where we reset the bit.
> >
> > We currently do this only for i.MX8M as Exynos uses a different
> > init flow where samsung_dsim_init() is called from
> > samsung_dsim_host_transfer().
> >
> > [1] 
> > https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
> >
> > Signed-off-by: Frieder Schrempf 
> > ---
> > Changes for v2:
> > * Drop RFC
> > ---
> >  drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++--
> >  1 file changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
> > b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index e0a402a85787..9775779721d9 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -859,6 +859,10 @@ static int samsung_dsim_init_link(struct 
> > samsung_dsim *dsi)
> > reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> > reg &= ~DSIM_STOP_STATE_CNT_MASK;
> > reg |= 
> > DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
> > +
> > +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
> > +   reg |= DSIM_FORCE_STOP_STATE;
> > +
> > samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> >
> > reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
> > @@ -1340,6 +1344,9 @@ static void samsung_dsim_atomic_pre_enable(struct 
> > drm_bridge *bridge,
> > ret = samsung_dsim_init(dsi);
> > if (ret)
> > return;
> > +
> > +   samsung_dsim_set_display_mode(dsi);
> > +   samsung_dsim_set_display_enable(dsi, true);
> > }
> >  }
> >
> > @@ -1347,9 +1354,16 @@ static void samsung_dsim_atomic_enable(struct 
> > drm_bridge *bridge,
> >struct drm_bridge_state 
> > *old_bridge_state)
> >  {
> > struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> > +   u32 reg;
> >
> > -   samsung_dsim_set_display_mode(dsi);
> > -   samsung_dsim_set_display_enable(dsi, true);
> > +   if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> > +   samsung_dsim_set_display_mode(dsi);
> > +   samsung_dsim_set_display_enable(dsi, true);
> > +   } else {
> > +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> > +   reg &= ~DSIM_FORCE_STOP_STATE;
> > +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> > +   }
> >
> > dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
> >  }
> > @@ -1358,10 +1372,17 @@ static void samsung_dsim_atomic_disable(struct 
> > drm_bridge *bridge,
> > struct drm_bridge_state 
> > *old_bridge_state)
> >  {
> > struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> > +   u32 reg;
> >
> > if (!(dsi->state & DSIM_STATE_ENABLED))
> > return;
> >
> > +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> > +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> > +   reg |= DSIM_FORCE_STOP_STATE;
> > +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> > +   }
> > +
> > dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
> >  }
> >
> > --
> > 2.40.0
> >
> 
>  Hi Frieder,
> 
>  I found this patch to break mipi-dsi display on my board which has:
>   - FocalTech FT5406 10pt touch controller (with no interrupt)
>   - Powertip PH800480T013-IDF02 compatible panel
>   - Toshiba TC358762 compatible DSI to DBI bridge
>   - ATTINY based regulator used for backlight controller and panel enable
> 
>  I enable this via a dt overlay in a 

Re: [PATCH v2 1/2] drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec

2023-07-19 Thread Frieder Schrempf
Hi Tim,

On 19.07.23 01:03, Tim Harvey wrote:
> On Thu, Jul 13, 2023 at 3:01 AM Frieder Schrempf
>  wrote:
>>
>> Hi Tim,
>>
>> On 13.07.23 09:18, Frieder Schrempf wrote:
>>> Hi Tim,
>>>
>>> On 13.07.23 00:34, Tim Harvey wrote:
 On Wed, May 3, 2023 at 9:33 AM Frieder Schrempf  wrote:
>
> From: Frieder Schrempf 
>
> According to the documentation [1] the proper enable flow is:
>
> 1. Enable DSI link and keep data lanes in LP-11 (stop state)
> 2. Disable stop state to bring data lanes into HS mode
>
> Currently we do this all at once within enable(), which doesn't
> allow to meet the requirements of some downstream bridges.
>
> To fix this we now enable the DSI in pre_enable() and force it
> into stop state using the FORCE_STOP_STATE bit in the ESCMODE
> register until enable() is called where we reset the bit.
>
> We currently do this only for i.MX8M as Exynos uses a different
> init flow where samsung_dsim_init() is called from
> samsung_dsim_host_transfer().
>
> [1] 
> https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
>
> Signed-off-by: Frieder Schrempf 
> ---
> Changes for v2:
> * Drop RFC
> ---
>  drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
> b/drivers/gpu/drm/bridge/samsung-dsim.c
> index e0a402a85787..9775779721d9 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -859,6 +859,10 @@ static int samsung_dsim_init_link(struct 
> samsung_dsim *dsi)
> reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> reg &= ~DSIM_STOP_STATE_CNT_MASK;
> reg |= 
> DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
> +
> +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
> +   reg |= DSIM_FORCE_STOP_STATE;
> +
> samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>
> reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
> @@ -1340,6 +1344,9 @@ static void samsung_dsim_atomic_pre_enable(struct 
> drm_bridge *bridge,
> ret = samsung_dsim_init(dsi);
> if (ret)
> return;
> +
> +   samsung_dsim_set_display_mode(dsi);
> +   samsung_dsim_set_display_enable(dsi, true);
> }
>  }
>
> @@ -1347,9 +1354,16 @@ static void samsung_dsim_atomic_enable(struct 
> drm_bridge *bridge,
>struct drm_bridge_state 
> *old_bridge_state)
>  {
> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> +   u32 reg;
>
> -   samsung_dsim_set_display_mode(dsi);
> -   samsung_dsim_set_display_enable(dsi, true);
> +   if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> +   samsung_dsim_set_display_mode(dsi);
> +   samsung_dsim_set_display_enable(dsi, true);
> +   } else {
> +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> +   reg &= ~DSIM_FORCE_STOP_STATE;
> +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> +   }
>
> dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
>  }
> @@ -1358,10 +1372,17 @@ static void samsung_dsim_atomic_disable(struct 
> drm_bridge *bridge,
> struct drm_bridge_state 
> *old_bridge_state)
>  {
> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> +   u32 reg;
>
> if (!(dsi->state & DSIM_STATE_ENABLED))
> return;
>
> +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> +   reg |= DSIM_FORCE_STOP_STATE;
> +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> +   }
> +
> dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
>  }
>
> --
> 2.40.0
>

 Hi Frieder,

 I found this patch to break mipi-dsi display on my board which has:
  - FocalTech FT5406 10pt touch controller (with no interrupt)
  - Powertip PH800480T013-IDF02 compatible panel
  - Toshiba TC358762 compatible DSI to DBI bridge
  - ATTINY based regulator used for backlight controller and panel enable

 I enable this via a dt overlay in a pending patch
 imx8mm-venice-gw72xx-0x-rpidsi.dtso [1] which works on 6.4 but not
 6.5-rc1 which has this patch.

 The issue appears as:
 [6.110585] samsung-dsim 32e6.dsi: xfer timed out: 29 06 00 00
 64 01 05 00 00 00
 [6.326588] tc358762 32e6.dsi.0: error 

Re: [PATCH v2 1/2] drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec

2023-07-18 Thread Tim Harvey
On Thu, Jul 13, 2023 at 3:01 AM Frieder Schrempf
 wrote:
>
> Hi Tim,
>
> On 13.07.23 09:18, Frieder Schrempf wrote:
> > Hi Tim,
> >
> > On 13.07.23 00:34, Tim Harvey wrote:
> >> On Wed, May 3, 2023 at 9:33 AM Frieder Schrempf  wrote:
> >>>
> >>> From: Frieder Schrempf 
> >>>
> >>> According to the documentation [1] the proper enable flow is:
> >>>
> >>> 1. Enable DSI link and keep data lanes in LP-11 (stop state)
> >>> 2. Disable stop state to bring data lanes into HS mode
> >>>
> >>> Currently we do this all at once within enable(), which doesn't
> >>> allow to meet the requirements of some downstream bridges.
> >>>
> >>> To fix this we now enable the DSI in pre_enable() and force it
> >>> into stop state using the FORCE_STOP_STATE bit in the ESCMODE
> >>> register until enable() is called where we reset the bit.
> >>>
> >>> We currently do this only for i.MX8M as Exynos uses a different
> >>> init flow where samsung_dsim_init() is called from
> >>> samsung_dsim_host_transfer().
> >>>
> >>> [1] 
> >>> https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
> >>>
> >>> Signed-off-by: Frieder Schrempf 
> >>> ---
> >>> Changes for v2:
> >>> * Drop RFC
> >>> ---
> >>>  drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++--
> >>>  1 file changed, 23 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
> >>> b/drivers/gpu/drm/bridge/samsung-dsim.c
> >>> index e0a402a85787..9775779721d9 100644
> >>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> >>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> >>> @@ -859,6 +859,10 @@ static int samsung_dsim_init_link(struct 
> >>> samsung_dsim *dsi)
> >>> reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> >>> reg &= ~DSIM_STOP_STATE_CNT_MASK;
> >>> reg |= 
> >>> DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
> >>> +
> >>> +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
> >>> +   reg |= DSIM_FORCE_STOP_STATE;
> >>> +
> >>> samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> >>>
> >>> reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
> >>> @@ -1340,6 +1344,9 @@ static void samsung_dsim_atomic_pre_enable(struct 
> >>> drm_bridge *bridge,
> >>> ret = samsung_dsim_init(dsi);
> >>> if (ret)
> >>> return;
> >>> +
> >>> +   samsung_dsim_set_display_mode(dsi);
> >>> +   samsung_dsim_set_display_enable(dsi, true);
> >>> }
> >>>  }
> >>>
> >>> @@ -1347,9 +1354,16 @@ static void samsung_dsim_atomic_enable(struct 
> >>> drm_bridge *bridge,
> >>>struct drm_bridge_state 
> >>> *old_bridge_state)
> >>>  {
> >>> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> >>> +   u32 reg;
> >>>
> >>> -   samsung_dsim_set_display_mode(dsi);
> >>> -   samsung_dsim_set_display_enable(dsi, true);
> >>> +   if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> >>> +   samsung_dsim_set_display_mode(dsi);
> >>> +   samsung_dsim_set_display_enable(dsi, true);
> >>> +   } else {
> >>> +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> >>> +   reg &= ~DSIM_FORCE_STOP_STATE;
> >>> +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> >>> +   }
> >>>
> >>> dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
> >>>  }
> >>> @@ -1358,10 +1372,17 @@ static void samsung_dsim_atomic_disable(struct 
> >>> drm_bridge *bridge,
> >>> struct drm_bridge_state 
> >>> *old_bridge_state)
> >>>  {
> >>> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> >>> +   u32 reg;
> >>>
> >>> if (!(dsi->state & DSIM_STATE_ENABLED))
> >>> return;
> >>>
> >>> +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> >>> +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> >>> +   reg |= DSIM_FORCE_STOP_STATE;
> >>> +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> >>> +   }
> >>> +
> >>> dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
> >>>  }
> >>>
> >>> --
> >>> 2.40.0
> >>>
> >>
> >> Hi Frieder,
> >>
> >> I found this patch to break mipi-dsi display on my board which has:
> >>  - FocalTech FT5406 10pt touch controller (with no interrupt)
> >>  - Powertip PH800480T013-IDF02 compatible panel
> >>  - Toshiba TC358762 compatible DSI to DBI bridge
> >>  - ATTINY based regulator used for backlight controller and panel enable
> >>
> >> I enable this via a dt overlay in a pending patch
> >> imx8mm-venice-gw72xx-0x-rpidsi.dtso [1] which works on 6.4 but not
> >> 6.5-rc1 which has this patch.
> >>
> >> The issue appears as:
> >> [6.110585] samsung-dsim 32e6.dsi: xfer timed out: 29 06 00 00
> >> 64 01 05 00 00 00
> >> [6.326588] tc358762 32e6.dsi.0: error initializing bridge (-110)
> >>
> >> Instead of
> >> [

Re: [PATCH v2 1/2] drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec

2023-07-13 Thread Frieder Schrempf
Hi Tim,

On 13.07.23 09:18, Frieder Schrempf wrote:
> Hi Tim,
> 
> On 13.07.23 00:34, Tim Harvey wrote:
>> On Wed, May 3, 2023 at 9:33 AM Frieder Schrempf  wrote:
>>>
>>> From: Frieder Schrempf 
>>>
>>> According to the documentation [1] the proper enable flow is:
>>>
>>> 1. Enable DSI link and keep data lanes in LP-11 (stop state)
>>> 2. Disable stop state to bring data lanes into HS mode
>>>
>>> Currently we do this all at once within enable(), which doesn't
>>> allow to meet the requirements of some downstream bridges.
>>>
>>> To fix this we now enable the DSI in pre_enable() and force it
>>> into stop state using the FORCE_STOP_STATE bit in the ESCMODE
>>> register until enable() is called where we reset the bit.
>>>
>>> We currently do this only for i.MX8M as Exynos uses a different
>>> init flow where samsung_dsim_init() is called from
>>> samsung_dsim_host_transfer().
>>>
>>> [1] 
>>> https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
>>>
>>> Signed-off-by: Frieder Schrempf 
>>> ---
>>> Changes for v2:
>>> * Drop RFC
>>> ---
>>>  drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++--
>>>  1 file changed, 23 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
>>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> index e0a402a85787..9775779721d9 100644
>>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> @@ -859,6 +859,10 @@ static int samsung_dsim_init_link(struct samsung_dsim 
>>> *dsi)
>>> reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
>>> reg &= ~DSIM_STOP_STATE_CNT_MASK;
>>> reg |= DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
>>> +
>>> +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
>>> +   reg |= DSIM_FORCE_STOP_STATE;
>>> +
>>> samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>>>
>>> reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
>>> @@ -1340,6 +1344,9 @@ static void samsung_dsim_atomic_pre_enable(struct 
>>> drm_bridge *bridge,
>>> ret = samsung_dsim_init(dsi);
>>> if (ret)
>>> return;
>>> +
>>> +   samsung_dsim_set_display_mode(dsi);
>>> +   samsung_dsim_set_display_enable(dsi, true);
>>> }
>>>  }
>>>
>>> @@ -1347,9 +1354,16 @@ static void samsung_dsim_atomic_enable(struct 
>>> drm_bridge *bridge,
>>>struct drm_bridge_state 
>>> *old_bridge_state)
>>>  {
>>> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
>>> +   u32 reg;
>>>
>>> -   samsung_dsim_set_display_mode(dsi);
>>> -   samsung_dsim_set_display_enable(dsi, true);
>>> +   if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
>>> +   samsung_dsim_set_display_mode(dsi);
>>> +   samsung_dsim_set_display_enable(dsi, true);
>>> +   } else {
>>> +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
>>> +   reg &= ~DSIM_FORCE_STOP_STATE;
>>> +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>>> +   }
>>>
>>> dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
>>>  }
>>> @@ -1358,10 +1372,17 @@ static void samsung_dsim_atomic_disable(struct 
>>> drm_bridge *bridge,
>>> struct drm_bridge_state 
>>> *old_bridge_state)
>>>  {
>>> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
>>> +   u32 reg;
>>>
>>> if (!(dsi->state & DSIM_STATE_ENABLED))
>>> return;
>>>
>>> +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
>>> +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
>>> +   reg |= DSIM_FORCE_STOP_STATE;
>>> +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>>> +   }
>>> +
>>> dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
>>>  }
>>>
>>> --
>>> 2.40.0
>>>
>>
>> Hi Frieder,
>>
>> I found this patch to break mipi-dsi display on my board which has:
>>  - FocalTech FT5406 10pt touch controller (with no interrupt)
>>  - Powertip PH800480T013-IDF02 compatible panel
>>  - Toshiba TC358762 compatible DSI to DBI bridge
>>  - ATTINY based regulator used for backlight controller and panel enable
>>
>> I enable this via a dt overlay in a pending patch
>> imx8mm-venice-gw72xx-0x-rpidsi.dtso [1] which works on 6.4 but not
>> 6.5-rc1 which has this patch.
>>
>> The issue appears as:
>> [6.110585] samsung-dsim 32e6.dsi: xfer timed out: 29 06 00 00
>> 64 01 05 00 00 00
>> [6.326588] tc358762 32e6.dsi.0: error initializing bridge (-110)
>>
>> Instead of
>> [1.011729] samsung-dsim 32e1.dsi: supply vddcore not found,
>> using dummy regulator
>> [1.019829] samsung-dsim 32e1.dsi: supply vddio not found,
>> using dummy regulator
>> [5.649928] samsung-dsim 32e1.dsi:
>> [drm:samsung_dsim_host_attach] Attached tc358762 device
>>
>> I'm curious what board/panel were 

Re: [PATCH v2 1/2] drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec

2023-07-13 Thread Frieder Schrempf
Hi Tim,

On 13.07.23 00:34, Tim Harvey wrote:
> On Wed, May 3, 2023 at 9:33 AM Frieder Schrempf  wrote:
>>
>> From: Frieder Schrempf 
>>
>> According to the documentation [1] the proper enable flow is:
>>
>> 1. Enable DSI link and keep data lanes in LP-11 (stop state)
>> 2. Disable stop state to bring data lanes into HS mode
>>
>> Currently we do this all at once within enable(), which doesn't
>> allow to meet the requirements of some downstream bridges.
>>
>> To fix this we now enable the DSI in pre_enable() and force it
>> into stop state using the FORCE_STOP_STATE bit in the ESCMODE
>> register until enable() is called where we reset the bit.
>>
>> We currently do this only for i.MX8M as Exynos uses a different
>> init flow where samsung_dsim_init() is called from
>> samsung_dsim_host_transfer().
>>
>> [1] 
>> https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
>>
>> Signed-off-by: Frieder Schrempf 
>> ---
>> Changes for v2:
>> * Drop RFC
>> ---
>>  drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++--
>>  1 file changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>> index e0a402a85787..9775779721d9 100644
>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>> @@ -859,6 +859,10 @@ static int samsung_dsim_init_link(struct samsung_dsim 
>> *dsi)
>> reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
>> reg &= ~DSIM_STOP_STATE_CNT_MASK;
>> reg |= DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
>> +
>> +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
>> +   reg |= DSIM_FORCE_STOP_STATE;
>> +
>> samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>>
>> reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
>> @@ -1340,6 +1344,9 @@ static void samsung_dsim_atomic_pre_enable(struct 
>> drm_bridge *bridge,
>> ret = samsung_dsim_init(dsi);
>> if (ret)
>> return;
>> +
>> +   samsung_dsim_set_display_mode(dsi);
>> +   samsung_dsim_set_display_enable(dsi, true);
>> }
>>  }
>>
>> @@ -1347,9 +1354,16 @@ static void samsung_dsim_atomic_enable(struct 
>> drm_bridge *bridge,
>>struct drm_bridge_state 
>> *old_bridge_state)
>>  {
>> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
>> +   u32 reg;
>>
>> -   samsung_dsim_set_display_mode(dsi);
>> -   samsung_dsim_set_display_enable(dsi, true);
>> +   if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
>> +   samsung_dsim_set_display_mode(dsi);
>> +   samsung_dsim_set_display_enable(dsi, true);
>> +   } else {
>> +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
>> +   reg &= ~DSIM_FORCE_STOP_STATE;
>> +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>> +   }
>>
>> dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
>>  }
>> @@ -1358,10 +1372,17 @@ static void samsung_dsim_atomic_disable(struct 
>> drm_bridge *bridge,
>> struct drm_bridge_state 
>> *old_bridge_state)
>>  {
>> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
>> +   u32 reg;
>>
>> if (!(dsi->state & DSIM_STATE_ENABLED))
>> return;
>>
>> +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
>> +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
>> +   reg |= DSIM_FORCE_STOP_STATE;
>> +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>> +   }
>> +
>> dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
>>  }
>>
>> --
>> 2.40.0
>>
> 
> Hi Frieder,
> 
> I found this patch to break mipi-dsi display on my board which has:
>  - FocalTech FT5406 10pt touch controller (with no interrupt)
>  - Powertip PH800480T013-IDF02 compatible panel
>  - Toshiba TC358762 compatible DSI to DBI bridge
>  - ATTINY based regulator used for backlight controller and panel enable
> 
> I enable this via a dt overlay in a pending patch
> imx8mm-venice-gw72xx-0x-rpidsi.dtso [1] which works on 6.4 but not
> 6.5-rc1 which has this patch.
> 
> The issue appears as:
> [6.110585] samsung-dsim 32e6.dsi: xfer timed out: 29 06 00 00
> 64 01 05 00 00 00
> [6.326588] tc358762 32e6.dsi.0: error initializing bridge (-110)
> 
> Instead of
> [1.011729] samsung-dsim 32e1.dsi: supply vddcore not found,
> using dummy regulator
> [1.019829] samsung-dsim 32e1.dsi: supply vddio not found,
> using dummy regulator
> [5.649928] samsung-dsim 32e1.dsi:
> [drm:samsung_dsim_host_attach] Attached tc358762 device
> 
> I'm curious what board/panel were you needing this for and do you have
> any ideas why it broke my setup?
> 
> I'm also curious what board/panel Alexander tested this with and if
> Adam or Jagan (or others) have 

Re: [PATCH v2 1/2] drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec

2023-07-13 Thread Alexander Stein
Hi,

Am Donnerstag, 13. Juli 2023, 00:34:47 CEST schrieb Tim Harvey:
> On Wed, May 3, 2023 at 9:33 AM Frieder Schrempf  wrote:
> > From: Frieder Schrempf 
> > 
> > According to the documentation [1] the proper enable flow is:
> > 
> > 1. Enable DSI link and keep data lanes in LP-11 (stop state)
> > 2. Disable stop state to bring data lanes into HS mode
> > 
> > Currently we do this all at once within enable(), which doesn't
> > allow to meet the requirements of some downstream bridges.
> > 
> > To fix this we now enable the DSI in pre_enable() and force it
> > into stop state using the FORCE_STOP_STATE bit in the ESCMODE
> > register until enable() is called where we reset the bit.
> > 
> > We currently do this only for i.MX8M as Exynos uses a different
> > init flow where samsung_dsim_init() is called from
> > samsung_dsim_host_transfer().
> > 
> > [1]
> > https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operatio
> > n
> > 
> > Signed-off-by: Frieder Schrempf 
> > ---
> > Changes for v2:
> > * Drop RFC
> > ---
> > 
> >  drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++--
> >  1 file changed, 23 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> > b/drivers/gpu/drm/bridge/samsung-dsim.c index e0a402a85787..9775779721d9
> > 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -859,6 +859,10 @@ static int samsung_dsim_init_link(struct samsung_dsim
> > *dsi)> 
> > reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> > reg &= ~DSIM_STOP_STATE_CNT_MASK;
> > reg |=
> > DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
> > 
> > +
> > +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
> > +   reg |= DSIM_FORCE_STOP_STATE;
> > +
> > 
> > samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> > 
> > reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
> > 
> > @@ -1340,6 +1344,9 @@ static void samsung_dsim_atomic_pre_enable(struct
> > drm_bridge *bridge,> 
> > ret = samsung_dsim_init(dsi);
> > if (ret)
> > 
> > return;
> > 
> > +
> > +   samsung_dsim_set_display_mode(dsi);
> > +   samsung_dsim_set_display_enable(dsi, true);
> > 
> > }
> >  
> >  }
> > 
> > @@ -1347,9 +1354,16 @@ static void samsung_dsim_atomic_enable(struct
> > drm_bridge *bridge,> 
> >struct drm_bridge_state
> >*old_bridge_state)
> >  
> >  {
> >  
> > struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> > 
> > +   u32 reg;
> > 
> > -   samsung_dsim_set_display_mode(dsi);
> > -   samsung_dsim_set_display_enable(dsi, true);
> > +   if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> > +   samsung_dsim_set_display_mode(dsi);
> > +   samsung_dsim_set_display_enable(dsi, true);
> > +   } else {
> > +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> > +   reg &= ~DSIM_FORCE_STOP_STATE;
> > +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> > +   }
> > 
> > dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
> >  
> >  }
> > 
> > @@ -1358,10 +1372,17 @@ static void samsung_dsim_atomic_disable(struct
> > drm_bridge *bridge,> 
> > struct drm_bridge_state
> > *old_bridge_state)
> >  
> >  {
> >  
> > struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> > 
> > +   u32 reg;
> > 
> > if (!(dsi->state & DSIM_STATE_ENABLED))
> > 
> > return;
> > 
> > +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> > +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> > +   reg |= DSIM_FORCE_STOP_STATE;
> > +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> > +   }
> > +
> > 
> > dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
> >  
> >  }
> > 
> > --
> > 2.40.0
> 
> Hi Frieder,
> 
> I found this patch to break mipi-dsi display on my board which has:
>  - FocalTech FT5406 10pt touch controller (with no interrupt)
>  - Powertip PH800480T013-IDF02 compatible panel
>  - Toshiba TC358762 compatible DSI to DBI bridge
>  - ATTINY based regulator used for backlight controller and panel enable
> 
> I enable this via a dt overlay in a pending patch
> imx8mm-venice-gw72xx-0x-rpidsi.dtso [1] which works on 6.4 but not
> 6.5-rc1 which has this patch.
> 
> The issue appears as:
> [6.110585] samsung-dsim 32e6.dsi: xfer timed out: 29 06 00 00
> 64 01 05 00 00 00
> [6.326588] tc358762 32e6.dsi.0: error initializing bridge (-110)
> 
> Instead of
> [1.011729] samsung-dsim 32e1.dsi: supply vddcore not found,
> using dummy regulator
> [1.019829] samsung-dsim 32e1.dsi: supply 

Re: [PATCH v2 1/2] drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec

2023-07-12 Thread Adam Ford
On Wed, Jul 12, 2023 at 5:34 PM Tim Harvey  wrote:
>
> On Wed, May 3, 2023 at 9:33 AM Frieder Schrempf  wrote:
> >
> > From: Frieder Schrempf 
> >
> > According to the documentation [1] the proper enable flow is:
> >
> > 1. Enable DSI link and keep data lanes in LP-11 (stop state)
> > 2. Disable stop state to bring data lanes into HS mode
> >
> > Currently we do this all at once within enable(), which doesn't
> > allow to meet the requirements of some downstream bridges.
> >
> > To fix this we now enable the DSI in pre_enable() and force it
> > into stop state using the FORCE_STOP_STATE bit in the ESCMODE
> > register until enable() is called where we reset the bit.
> >
> > We currently do this only for i.MX8M as Exynos uses a different
> > init flow where samsung_dsim_init() is called from
> > samsung_dsim_host_transfer().
> >
> > [1] 
> > https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
> >
> > Signed-off-by: Frieder Schrempf 
> > ---
> > Changes for v2:
> > * Drop RFC
> > ---
> >  drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++--
> >  1 file changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
> > b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index e0a402a85787..9775779721d9 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -859,6 +859,10 @@ static int samsung_dsim_init_link(struct samsung_dsim 
> > *dsi)
> > reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> > reg &= ~DSIM_STOP_STATE_CNT_MASK;
> > reg |= DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
> > +
> > +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
> > +   reg |= DSIM_FORCE_STOP_STATE;
> > +
> > samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> >
> > reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
> > @@ -1340,6 +1344,9 @@ static void samsung_dsim_atomic_pre_enable(struct 
> > drm_bridge *bridge,
> > ret = samsung_dsim_init(dsi);
> > if (ret)
> > return;
> > +
> > +   samsung_dsim_set_display_mode(dsi);
> > +   samsung_dsim_set_display_enable(dsi, true);
> > }
> >  }
> >
> > @@ -1347,9 +1354,16 @@ static void samsung_dsim_atomic_enable(struct 
> > drm_bridge *bridge,
> >struct drm_bridge_state 
> > *old_bridge_state)
> >  {
> > struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> > +   u32 reg;
> >
> > -   samsung_dsim_set_display_mode(dsi);
> > -   samsung_dsim_set_display_enable(dsi, true);
> > +   if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> > +   samsung_dsim_set_display_mode(dsi);
> > +   samsung_dsim_set_display_enable(dsi, true);
> > +   } else {
> > +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> > +   reg &= ~DSIM_FORCE_STOP_STATE;
> > +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> > +   }
> >
> > dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
> >  }
> > @@ -1358,10 +1372,17 @@ static void samsung_dsim_atomic_disable(struct 
> > drm_bridge *bridge,
> > struct drm_bridge_state 
> > *old_bridge_state)
> >  {
> > struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> > +   u32 reg;
> >
> > if (!(dsi->state & DSIM_STATE_ENABLED))
> > return;
> >
> > +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> > +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> > +   reg |= DSIM_FORCE_STOP_STATE;
> > +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> > +   }
> > +
> > dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
> >  }
> >
> > --
> > 2.40.0
> >
>
> Hi Frieder,
>
> I found this patch to break mipi-dsi display on my board which has:
>  - FocalTech FT5406 10pt touch controller (with no interrupt)
>  - Powertip PH800480T013-IDF02 compatible panel
>  - Toshiba TC358762 compatible DSI to DBI bridge
>  - ATTINY based regulator used for backlight controller and panel enable
>
> I enable this via a dt overlay in a pending patch
> imx8mm-venice-gw72xx-0x-rpidsi.dtso [1] which works on 6.4 but not
> 6.5-rc1 which has this patch.
>
> The issue appears as:
> [6.110585] samsung-dsim 32e6.dsi: xfer timed out: 29 06 00 00
> 64 01 05 00 00 00
> [6.326588] tc358762 32e6.dsi.0: error initializing bridge (-110)
>
> Instead of
> [1.011729] samsung-dsim 32e1.dsi: supply vddcore not found,
> using dummy regulator
> [1.019829] samsung-dsim 32e1.dsi: supply vddio not found,
> using dummy regulator
> [5.649928] samsung-dsim 32e1.dsi:
> [drm:samsung_dsim_host_attach] Attached tc358762 device
>
> I'm curious what board/panel were you needing this for and do you have
> any ideas why it broke my setup?
>
> I'm 

Re: [PATCH v2 1/2] drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec

2023-07-12 Thread Tim Harvey
On Wed, May 3, 2023 at 9:33 AM Frieder Schrempf  wrote:
>
> From: Frieder Schrempf 
>
> According to the documentation [1] the proper enable flow is:
>
> 1. Enable DSI link and keep data lanes in LP-11 (stop state)
> 2. Disable stop state to bring data lanes into HS mode
>
> Currently we do this all at once within enable(), which doesn't
> allow to meet the requirements of some downstream bridges.
>
> To fix this we now enable the DSI in pre_enable() and force it
> into stop state using the FORCE_STOP_STATE bit in the ESCMODE
> register until enable() is called where we reset the bit.
>
> We currently do this only for i.MX8M as Exynos uses a different
> init flow where samsung_dsim_init() is called from
> samsung_dsim_host_transfer().
>
> [1] https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
>
> Signed-off-by: Frieder Schrempf 
> ---
> Changes for v2:
> * Drop RFC
> ---
>  drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
> b/drivers/gpu/drm/bridge/samsung-dsim.c
> index e0a402a85787..9775779721d9 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -859,6 +859,10 @@ static int samsung_dsim_init_link(struct samsung_dsim 
> *dsi)
> reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> reg &= ~DSIM_STOP_STATE_CNT_MASK;
> reg |= DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
> +
> +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
> +   reg |= DSIM_FORCE_STOP_STATE;
> +
> samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
>
> reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
> @@ -1340,6 +1344,9 @@ static void samsung_dsim_atomic_pre_enable(struct 
> drm_bridge *bridge,
> ret = samsung_dsim_init(dsi);
> if (ret)
> return;
> +
> +   samsung_dsim_set_display_mode(dsi);
> +   samsung_dsim_set_display_enable(dsi, true);
> }
>  }
>
> @@ -1347,9 +1354,16 @@ static void samsung_dsim_atomic_enable(struct 
> drm_bridge *bridge,
>struct drm_bridge_state 
> *old_bridge_state)
>  {
> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> +   u32 reg;
>
> -   samsung_dsim_set_display_mode(dsi);
> -   samsung_dsim_set_display_enable(dsi, true);
> +   if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> +   samsung_dsim_set_display_mode(dsi);
> +   samsung_dsim_set_display_enable(dsi, true);
> +   } else {
> +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> +   reg &= ~DSIM_FORCE_STOP_STATE;
> +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> +   }
>
> dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
>  }
> @@ -1358,10 +1372,17 @@ static void samsung_dsim_atomic_disable(struct 
> drm_bridge *bridge,
> struct drm_bridge_state 
> *old_bridge_state)
>  {
> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> +   u32 reg;
>
> if (!(dsi->state & DSIM_STATE_ENABLED))
> return;
>
> +   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> +   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> +   reg |= DSIM_FORCE_STOP_STATE;
> +   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> +   }
> +
> dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
>  }
>
> --
> 2.40.0
>

Hi Frieder,

I found this patch to break mipi-dsi display on my board which has:
 - FocalTech FT5406 10pt touch controller (with no interrupt)
 - Powertip PH800480T013-IDF02 compatible panel
 - Toshiba TC358762 compatible DSI to DBI bridge
 - ATTINY based regulator used for backlight controller and panel enable

I enable this via a dt overlay in a pending patch
imx8mm-venice-gw72xx-0x-rpidsi.dtso [1] which works on 6.4 but not
6.5-rc1 which has this patch.

The issue appears as:
[6.110585] samsung-dsim 32e6.dsi: xfer timed out: 29 06 00 00
64 01 05 00 00 00
[6.326588] tc358762 32e6.dsi.0: error initializing bridge (-110)

Instead of
[1.011729] samsung-dsim 32e1.dsi: supply vddcore not found,
using dummy regulator
[1.019829] samsung-dsim 32e1.dsi: supply vddio not found,
using dummy regulator
[5.649928] samsung-dsim 32e1.dsi:
[drm:samsung_dsim_host_attach] Attached tc358762 device

I'm curious what board/panel were you needing this for and do you have
any ideas why it broke my setup?

I'm also curious what board/panel Alexander tested this with and if
Adam or Jagan (or others) have tested this with their hardware?

best regards,

Tim
[1] 
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20230711221124.2127186-1-thar...@gateworks.com/


Re: [PATCH v2 1/2] drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec

2023-05-16 Thread Neil Armstrong

On 03/05/2023 18:33, Frieder Schrempf wrote:

From: Frieder Schrempf 

According to the documentation [1] the proper enable flow is:

1. Enable DSI link and keep data lanes in LP-11 (stop state)
2. Disable stop state to bring data lanes into HS mode

Currently we do this all at once within enable(), which doesn't
allow to meet the requirements of some downstream bridges.

To fix this we now enable the DSI in pre_enable() and force it
into stop state using the FORCE_STOP_STATE bit in the ESCMODE
register until enable() is called where we reset the bit.

We currently do this only for i.MX8M as Exynos uses a different
init flow where samsung_dsim_init() is called from
samsung_dsim_host_transfer().

[1] https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation

Signed-off-by: Frieder Schrempf 
---
Changes for v2:
* Drop RFC
---
  drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++--
  1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index e0a402a85787..9775779721d9 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -859,6 +859,10 @@ static int samsung_dsim_init_link(struct samsung_dsim *dsi)
reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
reg &= ~DSIM_STOP_STATE_CNT_MASK;
reg |= DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
+
+   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
+   reg |= DSIM_FORCE_STOP_STATE;
+
samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
  
  	reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);

@@ -1340,6 +1344,9 @@ static void samsung_dsim_atomic_pre_enable(struct 
drm_bridge *bridge,
ret = samsung_dsim_init(dsi);
if (ret)
return;
+
+   samsung_dsim_set_display_mode(dsi);
+   samsung_dsim_set_display_enable(dsi, true);
}
  }
  
@@ -1347,9 +1354,16 @@ static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,

   struct drm_bridge_state 
*old_bridge_state)
  {
struct samsung_dsim *dsi = bridge_to_dsi(bridge);
+   u32 reg;
  
-	samsung_dsim_set_display_mode(dsi);

-   samsung_dsim_set_display_enable(dsi, true);
+   if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
+   samsung_dsim_set_display_mode(dsi);
+   samsung_dsim_set_display_enable(dsi, true);
+   } else {
+   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
+   reg &= ~DSIM_FORCE_STOP_STATE;
+   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
+   }
  
  	dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;

  }
@@ -1358,10 +1372,17 @@ static void samsung_dsim_atomic_disable(struct 
drm_bridge *bridge,
struct drm_bridge_state 
*old_bridge_state)
  {
struct samsung_dsim *dsi = bridge_to_dsi(bridge);
+   u32 reg;
  
  	if (!(dsi->state & DSIM_STATE_ENABLED))

return;
  
+	if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {

+   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
+   reg |= DSIM_FORCE_STOP_STATE;
+   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
+   }
+
dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
  }
  


Reviewed-by: Neil Armstrong 


Re: [PATCH v2 1/2] drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec

2023-05-04 Thread Alexander Stein
Hello Frieder,

Am Mittwoch, 3. Mai 2023, 18:33:06 CEST schrieb Frieder Schrempf:
> From: Frieder Schrempf 
> 
> According to the documentation [1] the proper enable flow is:
> 
> 1. Enable DSI link and keep data lanes in LP-11 (stop state)
> 2. Disable stop state to bring data lanes into HS mode
> 
> Currently we do this all at once within enable(), which doesn't
> allow to meet the requirements of some downstream bridges.
> 
> To fix this we now enable the DSI in pre_enable() and force it
> into stop state using the FORCE_STOP_STATE bit in the ESCMODE
> register until enable() is called where we reset the bit.
> 
> We currently do this only for i.MX8M as Exynos uses a different
> init flow where samsung_dsim_init() is called from
> samsung_dsim_host_transfer().
> 
> [1]
> https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
> 
> Signed-off-by: Frieder Schrempf 
> ---
> Changes for v2:
> * Drop RFC
> ---
>  drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> b/drivers/gpu/drm/bridge/samsung-dsim.c index e0a402a85787..9775779721d9
> 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -859,6 +859,10 @@ static int samsung_dsim_init_link(struct samsung_dsim
> *dsi) reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
>   reg &= ~DSIM_STOP_STATE_CNT_MASK;
>   reg |= DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
> +
> + if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
> + reg |= DSIM_FORCE_STOP_STATE;
> +
>   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> 
>   reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
> @@ -1340,6 +1344,9 @@ static void samsung_dsim_atomic_pre_enable(struct
> drm_bridge *bridge, ret = samsung_dsim_init(dsi);
>   if (ret)
>   return;
> +
> + samsung_dsim_set_display_mode(dsi);
> + samsung_dsim_set_display_enable(dsi, true);
>   }
>  }
> 
> @@ -1347,9 +1354,16 @@ static void samsung_dsim_atomic_enable(struct
> drm_bridge *bridge, struct drm_bridge_state *old_bridge_state)
>  {
>   struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> + u32 reg;
> 
> - samsung_dsim_set_display_mode(dsi);
> - samsung_dsim_set_display_enable(dsi, true);
> + if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> + samsung_dsim_set_display_mode(dsi);
> + samsung_dsim_set_display_enable(dsi, true);
> + } else {
> + reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> + reg &= ~DSIM_FORCE_STOP_STATE;
> + samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> + }
> 
>   dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
>  }
> @@ -1358,10 +1372,17 @@ static void samsung_dsim_atomic_disable(struct
> drm_bridge *bridge, struct drm_bridge_state *old_bridge_state)
>  {
>   struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> + u32 reg;
> 
>   if (!(dsi->state & DSIM_STATE_ENABLED))
>   return;
> 
> + if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> + reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> + reg |= DSIM_FORCE_STOP_STATE;
> + samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> + }
> +
>   dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
>  }

I know that this is necessary right now, but I don't like that 
'samsung_dsim_hw_is_exynos()' checks all over the place.

Despite that:
Tested-by: Alexander Stein  #TQMa8MxML/MBa8Mx

-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/




[PATCH v2 1/2] drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec

2023-05-03 Thread Frieder Schrempf
From: Frieder Schrempf 

According to the documentation [1] the proper enable flow is:

1. Enable DSI link and keep data lanes in LP-11 (stop state)
2. Disable stop state to bring data lanes into HS mode

Currently we do this all at once within enable(), which doesn't
allow to meet the requirements of some downstream bridges.

To fix this we now enable the DSI in pre_enable() and force it
into stop state using the FORCE_STOP_STATE bit in the ESCMODE
register until enable() is called where we reset the bit.

We currently do this only for i.MX8M as Exynos uses a different
init flow where samsung_dsim_init() is called from
samsung_dsim_host_transfer().

[1] https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation

Signed-off-by: Frieder Schrempf 
---
Changes for v2:
* Drop RFC
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index e0a402a85787..9775779721d9 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -859,6 +859,10 @@ static int samsung_dsim_init_link(struct samsung_dsim *dsi)
reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
reg &= ~DSIM_STOP_STATE_CNT_MASK;
reg |= DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
+
+   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
+   reg |= DSIM_FORCE_STOP_STATE;
+
samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
 
reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
@@ -1340,6 +1344,9 @@ static void samsung_dsim_atomic_pre_enable(struct 
drm_bridge *bridge,
ret = samsung_dsim_init(dsi);
if (ret)
return;
+
+   samsung_dsim_set_display_mode(dsi);
+   samsung_dsim_set_display_enable(dsi, true);
}
 }
 
@@ -1347,9 +1354,16 @@ static void samsung_dsim_atomic_enable(struct drm_bridge 
*bridge,
   struct drm_bridge_state 
*old_bridge_state)
 {
struct samsung_dsim *dsi = bridge_to_dsi(bridge);
+   u32 reg;
 
-   samsung_dsim_set_display_mode(dsi);
-   samsung_dsim_set_display_enable(dsi, true);
+   if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
+   samsung_dsim_set_display_mode(dsi);
+   samsung_dsim_set_display_enable(dsi, true);
+   } else {
+   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
+   reg &= ~DSIM_FORCE_STOP_STATE;
+   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
+   }
 
dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
 }
@@ -1358,10 +1372,17 @@ static void samsung_dsim_atomic_disable(struct 
drm_bridge *bridge,
struct drm_bridge_state 
*old_bridge_state)
 {
struct samsung_dsim *dsi = bridge_to_dsi(bridge);
+   u32 reg;
 
if (!(dsi->state & DSIM_STATE_ENABLED))
return;
 
+   if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
+   reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
+   reg |= DSIM_FORCE_STOP_STATE;
+   samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
+   }
+
dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
 }
 
-- 
2.40.0