Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-29 Thread Abhinav Kumar




On 8/29/2023 7:05 PM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 22:21, Abhinav Kumar  wrote:




On 8/29/2023 12:15 PM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 22:09, Abhinav Kumar  wrote:




On 8/29/2023 11:51 AM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 20:22, Abhinav Kumar  wrote:




On 8/29/2023 9:43 AM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 19:37, Abhinav Kumar  wrote:




On 8/29/2023 2:26 AM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 12:22,  wrote:


On 28/08/2023 19:07, Abhinav Kumar wrote:

Hi Neil

Sorry I didnt respond earlier on this thread.

On 8/28/2023 1:49 AM, neil.armstr...@linaro.org wrote:

Hi Jessica,

On 25/08/2023 20:37, Jessica Zhang wrote:



On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:

Hi Maxime,

On 21/08/2023 10:17, Maxime Ripard wrote:

Hi,

On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org wrote:

On 17/08/2023 20:35, Dmitry Baryshkov wrote:

On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:

Sending HS commands will always work on any controller, it's all
about LP commands. The Samsung panels you listed only send HS
commands so they can use prepare_prev_first and work on any
controllers.


I think there is some misunderstanding there, supported by the
description of the flag.

If I remember correctly, some hosts (sunxi) can not send DCS
commands after enabling video stream and switching to HS mode, see
[1]. Thus, as you know, most of the drivers have all DSI panel setup
commands in drm_panel_funcs::prepare() /
drm_bridge_funcs::pre_enable() callbacks, not paying attention
whether these commands are to be sent in LP or in HS mode.

Previously DSI source drivers could power on the DSI link either in
mode_set() or in pre_enable() callbacks, with mode_set() being the
hack to make panel/bridge drivers to be able to send commands from
their prepare() / pre_enable() callbacks.

With the prev_first flags being introduced, we have established that
DSI link should be enabled in DSI host's pre_enable() callback and
switched to HS mode (be it command or video) in the enable()
callback.

So far so good.


It seems coherent, I would like first to have a state of all DSI host
drivers and make this would actually work first before adding the
prev_first flag to all the required panels.


This is definitely what we should do in an ideal world, but at least for
sunxi there's no easy way for it at the moment. There's no documentation
for it and the driver provided doesn't allow this to happen.

Note that I'm not trying to discourage you or something here, I'm simply
pointing out that this will be something that we will have to take into
account. And it's possible that other drivers are in a similar
situation.


Unfortunately this change is not fully backwards-compatible. This
requires that all DSI panels sending commands from prepare() should
have the prepare_prev_first flag. In some sense, all such patches
might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
flag to drm_panel").


This kind of migration should be done *before* any possible
regression, not the other way round.

If all panels sending commands from prepare() should have the
prepare_prev_first flag, then it should be first, check for
regressions then continue.





I understand, but this patch doesn't qualify as a fix for
9e15123eca79 and is too late to be merged in drm-misc-next for
v6.6, and since 9e15123eca79 actually breaks some support it
should be reverted (+ deps) since we are late in the rc cycles.


If we go this way, we can never reapply these patches. There will be
no guarantee that all panel drivers are completely converted. We
already have a story without an observable end -
DRM_BRIDGE_ATTACH_NO_CONNECTOR.


I don't understand this point, who would block re-applying the patches ?

The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
Linux version and went smoothly because we reverted regressing patches
and restarted when needed, I don't understand why we can't do this
here aswell.


I'd consider that the DSI driver is correct here and it is about the
panel drivers that require fixes patches. If you care about the
particular Fixes tag, I have provided one several lines above.


Unfortunately it should be done in the other way round, prepare for
migration, then migrate,

I mean if it's a required migration, then it should be done and I'll
support it from both bridge and panel PoV.

So, first this patch has the wrong Fixes tag, and I would like a
better explanation on the commit message in any case. Then I would
like to have an ack from some drm-misc maintainers before applying it
because it fixes a patch that was sent via the msm tree thus per the
drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.


Sorry, it's not clear to me what you'd like our feedback on exactly?


So let me resume the situation:

- pre_enable_prev_first was introduced in [1]
- some panels made use of pre_enable_prev_first
- 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-29 Thread Dmitry Baryshkov
On Tue, 29 Aug 2023 at 22:21, Abhinav Kumar  wrote:
>
>
>
> On 8/29/2023 12:15 PM, Dmitry Baryshkov wrote:
> > On Tue, 29 Aug 2023 at 22:09, Abhinav Kumar  
> > wrote:
> >>
> >>
> >>
> >> On 8/29/2023 11:51 AM, Dmitry Baryshkov wrote:
> >>> On Tue, 29 Aug 2023 at 20:22, Abhinav Kumar  
> >>> wrote:
> 
> 
> 
>  On 8/29/2023 9:43 AM, Dmitry Baryshkov wrote:
> > On Tue, 29 Aug 2023 at 19:37, Abhinav Kumar  
> > wrote:
> >>
> >>
> >>
> >> On 8/29/2023 2:26 AM, Dmitry Baryshkov wrote:
> >>> On Tue, 29 Aug 2023 at 12:22,  wrote:
> 
>  On 28/08/2023 19:07, Abhinav Kumar wrote:
> > Hi Neil
> >
> > Sorry I didnt respond earlier on this thread.
> >
> > On 8/28/2023 1:49 AM, neil.armstr...@linaro.org wrote:
> >> Hi Jessica,
> >>
> >> On 25/08/2023 20:37, Jessica Zhang wrote:
> >>>
> >>>
> >>> On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:
>  Hi Maxime,
> 
>  On 21/08/2023 10:17, Maxime Ripard wrote:
> > Hi,
> >
> > On Fri, Aug 18, 2023 at 10:25:48AM +0200, 
> > neil.armstr...@linaro.org wrote:
> >> On 17/08/2023 20:35, Dmitry Baryshkov wrote:
> >>> On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:
>  Sending HS commands will always work on any controller, it's 
>  all
>  about LP commands. The Samsung panels you listed only send HS
>  commands so they can use prepare_prev_first and work on any
>  controllers.
> >>>
> >>> I think there is some misunderstanding there, supported by the
> >>> description of the flag.
> >>>
> >>> If I remember correctly, some hosts (sunxi) can not send DCS
> >>> commands after enabling video stream and switching to HS 
> >>> mode, see
> >>> [1]. Thus, as you know, most of the drivers have all DSI 
> >>> panel setup
> >>> commands in drm_panel_funcs::prepare() /
> >>> drm_bridge_funcs::pre_enable() callbacks, not paying attention
> >>> whether these commands are to be sent in LP or in HS mode.
> >>>
> >>> Previously DSI source drivers could power on the DSI link 
> >>> either in
> >>> mode_set() or in pre_enable() callbacks, with mode_set() 
> >>> being the
> >>> hack to make panel/bridge drivers to be able to send commands 
> >>> from
> >>> their prepare() / pre_enable() callbacks.
> >>>
> >>> With the prev_first flags being introduced, we have 
> >>> established that
> >>> DSI link should be enabled in DSI host's pre_enable() 
> >>> callback and
> >>> switched to HS mode (be it command or video) in the enable()
> >>> callback.
> >>>
> >>> So far so good.
> >>
> >> It seems coherent, I would like first to have a state of all 
> >> DSI host
> >> drivers and make this would actually work first before adding 
> >> the
> >> prev_first flag to all the required panels.
> >
> > This is definitely what we should do in an ideal world, but at 
> > least for
> > sunxi there's no easy way for it at the moment. There's no 
> > documentation
> > for it and the driver provided doesn't allow this to happen.
> >
> > Note that I'm not trying to discourage you or something here, 
> > I'm simply
> > pointing out that this will be something that we will have to 
> > take into
> > account. And it's possible that other drivers are in a similar
> > situation.
> >
> >>> Unfortunately this change is not fully backwards-compatible. 
> >>> This
> >>> requires that all DSI panels sending commands from prepare() 
> >>> should
> >>> have the prepare_prev_first flag. In some sense, all such 
> >>> patches
> >>> might have Fixes: 5ea6b1702781 ("drm/panel: Add 
> >>> prepare_prev_first
> >>> flag to drm_panel").
> >>
> >> This kind of migration should be done *before* any possible
> >> regression, not the other way round.
> >>
> >> If all panels sending commands from prepare() should have the
> >> prepare_prev_first flag, then it should be first, check for
> >> regressions then continue.
> >>
> >> 
> >>
> 
>  I understand, but this patch doesn't 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-29 Thread Abhinav Kumar

Hi Dave

Nice to e-meet you.

On 8/29/2023 7:13 AM, Dave Stevenson wrote:

Hi Neil

On Mon, 28 Aug 2023 at 09:49,  wrote:


Hi Jessica,

On 25/08/2023 20:37, Jessica Zhang wrote:



On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:

Hi Maxime,

On 21/08/2023 10:17, Maxime Ripard wrote:

Hi,

On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org wrote:

On 17/08/2023 20:35, Dmitry Baryshkov wrote:

On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:

Sending HS commands will always work on any controller, it's all
about LP commands. The Samsung panels you listed only send HS
commands so they can use prepare_prev_first and work on any
controllers.


I think there is some misunderstanding there, supported by the
description of the flag.

If I remember correctly, some hosts (sunxi) can not send DCS
commands after enabling video stream and switching to HS mode, see
[1]. Thus, as you know, most of the drivers have all DSI panel setup
commands in drm_panel_funcs::prepare() /
drm_bridge_funcs::pre_enable() callbacks, not paying attention
whether these commands are to be sent in LP or in HS mode.

Previously DSI source drivers could power on the DSI link either in
mode_set() or in pre_enable() callbacks, with mode_set() being the
hack to make panel/bridge drivers to be able to send commands from
their prepare() / pre_enable() callbacks.

With the prev_first flags being introduced, we have established that
DSI link should be enabled in DSI host's pre_enable() callback and
switched to HS mode (be it command or video) in the enable()
callback.

So far so good.


It seems coherent, I would like first to have a state of all DSI host
drivers and make this would actually work first before adding the
prev_first flag to all the required panels.


This is definitely what we should do in an ideal world, but at least for
sunxi there's no easy way for it at the moment. There's no documentation
for it and the driver provided doesn't allow this to happen.

Note that I'm not trying to discourage you or something here, I'm simply
pointing out that this will be something that we will have to take into
account. And it's possible that other drivers are in a similar
situation.


Unfortunately this change is not fully backwards-compatible. This
requires that all DSI panels sending commands from prepare() should
have the prepare_prev_first flag. In some sense, all such patches
might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
flag to drm_panel").


This kind of migration should be done *before* any possible
regression, not the other way round.

If all panels sending commands from prepare() should have the
prepare_prev_first flag, then it should be first, check for
regressions then continue.





I understand, but this patch doesn't qualify as a fix for
9e15123eca79 and is too late to be merged in drm-misc-next for
v6.6, and since 9e15123eca79 actually breaks some support it
should be reverted (+ deps) since we are late in the rc cycles.


If we go this way, we can never reapply these patches. There will be
no guarantee that all panel drivers are completely converted. We
already have a story without an observable end -
DRM_BRIDGE_ATTACH_NO_CONNECTOR.


I don't understand this point, who would block re-applying the patches ?

The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
Linux version and went smoothly because we reverted regressing patches
and restarted when needed, I don't understand why we can't do this
here aswell.


I'd consider that the DSI driver is correct here and it is about the
panel drivers that require fixes patches. If you care about the
particular Fixes tag, I have provided one several lines above.


Unfortunately it should be done in the other way round, prepare for
migration, then migrate,

I mean if it's a required migration, then it should be done and I'll
support it from both bridge and panel PoV.

So, first this patch has the wrong Fixes tag, and I would like a
better explanation on the commit message in any case. Then I would
like to have an ack from some drm-misc maintainers before applying it
because it fixes a patch that was sent via the msm tree thus per the
drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.


Sorry, it's not clear to me what you'd like our feedback on exactly?


So let me resume the situation:

- pre_enable_prev_first was introduced in [1]
- some panels made use of pre_enable_prev_first
- Visionox VTDR6130 was enabled on SM8550 systems and works on v6.5 kernels and 
before
- patch [2] was introduced on MSM DRM tree, breaking VTDR6130 on SM8550 systems 
(and probably other Video mode panels on Qcom platforms)
- this fix was sent late, and is now too late to be merged via drm-misc-next


Hi Neil and Maxime,

I agree with Neil that 9e15123eca79 was the commit that introduced the issue 
(since it changed the MSM DSI host behavior).

However, I'm not too keen on simply reverting that patch because

1) it's not wrong to 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-29 Thread Abhinav Kumar




On 8/29/2023 12:15 PM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 22:09, Abhinav Kumar  wrote:




On 8/29/2023 11:51 AM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 20:22, Abhinav Kumar  wrote:




On 8/29/2023 9:43 AM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 19:37, Abhinav Kumar  wrote:




On 8/29/2023 2:26 AM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 12:22,  wrote:


On 28/08/2023 19:07, Abhinav Kumar wrote:

Hi Neil

Sorry I didnt respond earlier on this thread.

On 8/28/2023 1:49 AM, neil.armstr...@linaro.org wrote:

Hi Jessica,

On 25/08/2023 20:37, Jessica Zhang wrote:



On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:

Hi Maxime,

On 21/08/2023 10:17, Maxime Ripard wrote:

Hi,

On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org wrote:

On 17/08/2023 20:35, Dmitry Baryshkov wrote:

On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:

Sending HS commands will always work on any controller, it's all
about LP commands. The Samsung panels you listed only send HS
commands so they can use prepare_prev_first and work on any
controllers.


I think there is some misunderstanding there, supported by the
description of the flag.

If I remember correctly, some hosts (sunxi) can not send DCS
commands after enabling video stream and switching to HS mode, see
[1]. Thus, as you know, most of the drivers have all DSI panel setup
commands in drm_panel_funcs::prepare() /
drm_bridge_funcs::pre_enable() callbacks, not paying attention
whether these commands are to be sent in LP or in HS mode.

Previously DSI source drivers could power on the DSI link either in
mode_set() or in pre_enable() callbacks, with mode_set() being the
hack to make panel/bridge drivers to be able to send commands from
their prepare() / pre_enable() callbacks.

With the prev_first flags being introduced, we have established that
DSI link should be enabled in DSI host's pre_enable() callback and
switched to HS mode (be it command or video) in the enable()
callback.

So far so good.


It seems coherent, I would like first to have a state of all DSI host
drivers and make this would actually work first before adding the
prev_first flag to all the required panels.


This is definitely what we should do in an ideal world, but at least for
sunxi there's no easy way for it at the moment. There's no documentation
for it and the driver provided doesn't allow this to happen.

Note that I'm not trying to discourage you or something here, I'm simply
pointing out that this will be something that we will have to take into
account. And it's possible that other drivers are in a similar
situation.


Unfortunately this change is not fully backwards-compatible. This
requires that all DSI panels sending commands from prepare() should
have the prepare_prev_first flag. In some sense, all such patches
might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
flag to drm_panel").


This kind of migration should be done *before* any possible
regression, not the other way round.

If all panels sending commands from prepare() should have the
prepare_prev_first flag, then it should be first, check for
regressions then continue.





I understand, but this patch doesn't qualify as a fix for
9e15123eca79 and is too late to be merged in drm-misc-next for
v6.6, and since 9e15123eca79 actually breaks some support it
should be reverted (+ deps) since we are late in the rc cycles.


If we go this way, we can never reapply these patches. There will be
no guarantee that all panel drivers are completely converted. We
already have a story without an observable end -
DRM_BRIDGE_ATTACH_NO_CONNECTOR.


I don't understand this point, who would block re-applying the patches ?

The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
Linux version and went smoothly because we reverted regressing patches
and restarted when needed, I don't understand why we can't do this
here aswell.


I'd consider that the DSI driver is correct here and it is about the
panel drivers that require fixes patches. If you care about the
particular Fixes tag, I have provided one several lines above.


Unfortunately it should be done in the other way round, prepare for
migration, then migrate,

I mean if it's a required migration, then it should be done and I'll
support it from both bridge and panel PoV.

So, first this patch has the wrong Fixes tag, and I would like a
better explanation on the commit message in any case. Then I would
like to have an ack from some drm-misc maintainers before applying it
because it fixes a patch that was sent via the msm tree thus per the
drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.


Sorry, it's not clear to me what you'd like our feedback on exactly?


So let me resume the situation:

- pre_enable_prev_first was introduced in [1]
- some panels made use of pre_enable_prev_first
- Visionox VTDR6130 was enabled on SM8550 systems and works on v6.5 kernels and 
before
- patch [2] was 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-29 Thread Dmitry Baryshkov
On Tue, 29 Aug 2023 at 22:09, Abhinav Kumar  wrote:
>
>
>
> On 8/29/2023 11:51 AM, Dmitry Baryshkov wrote:
> > On Tue, 29 Aug 2023 at 20:22, Abhinav Kumar  
> > wrote:
> >>
> >>
> >>
> >> On 8/29/2023 9:43 AM, Dmitry Baryshkov wrote:
> >>> On Tue, 29 Aug 2023 at 19:37, Abhinav Kumar  
> >>> wrote:
> 
> 
> 
>  On 8/29/2023 2:26 AM, Dmitry Baryshkov wrote:
> > On Tue, 29 Aug 2023 at 12:22,  wrote:
> >>
> >> On 28/08/2023 19:07, Abhinav Kumar wrote:
> >>> Hi Neil
> >>>
> >>> Sorry I didnt respond earlier on this thread.
> >>>
> >>> On 8/28/2023 1:49 AM, neil.armstr...@linaro.org wrote:
>  Hi Jessica,
> 
>  On 25/08/2023 20:37, Jessica Zhang wrote:
> >
> >
> > On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:
> >> Hi Maxime,
> >>
> >> On 21/08/2023 10:17, Maxime Ripard wrote:
> >>> Hi,
> >>>
> >>> On Fri, Aug 18, 2023 at 10:25:48AM +0200, 
> >>> neil.armstr...@linaro.org wrote:
>  On 17/08/2023 20:35, Dmitry Baryshkov wrote:
> > On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:
> >> Sending HS commands will always work on any controller, it's 
> >> all
> >> about LP commands. The Samsung panels you listed only send HS
> >> commands so they can use prepare_prev_first and work on any
> >> controllers.
> >
> > I think there is some misunderstanding there, supported by the
> > description of the flag.
> >
> > If I remember correctly, some hosts (sunxi) can not send DCS
> > commands after enabling video stream and switching to HS mode, 
> > see
> > [1]. Thus, as you know, most of the drivers have all DSI panel 
> > setup
> > commands in drm_panel_funcs::prepare() /
> > drm_bridge_funcs::pre_enable() callbacks, not paying attention
> > whether these commands are to be sent in LP or in HS mode.
> >
> > Previously DSI source drivers could power on the DSI link 
> > either in
> > mode_set() or in pre_enable() callbacks, with mode_set() being 
> > the
> > hack to make panel/bridge drivers to be able to send commands 
> > from
> > their prepare() / pre_enable() callbacks.
> >
> > With the prev_first flags being introduced, we have established 
> > that
> > DSI link should be enabled in DSI host's pre_enable() callback 
> > and
> > switched to HS mode (be it command or video) in the enable()
> > callback.
> >
> > So far so good.
> 
>  It seems coherent, I would like first to have a state of all DSI 
>  host
>  drivers and make this would actually work first before adding the
>  prev_first flag to all the required panels.
> >>>
> >>> This is definitely what we should do in an ideal world, but at 
> >>> least for
> >>> sunxi there's no easy way for it at the moment. There's no 
> >>> documentation
> >>> for it and the driver provided doesn't allow this to happen.
> >>>
> >>> Note that I'm not trying to discourage you or something here, I'm 
> >>> simply
> >>> pointing out that this will be something that we will have to 
> >>> take into
> >>> account. And it's possible that other drivers are in a similar
> >>> situation.
> >>>
> > Unfortunately this change is not fully backwards-compatible. 
> > This
> > requires that all DSI panels sending commands from prepare() 
> > should
> > have the prepare_prev_first flag. In some sense, all such 
> > patches
> > might have Fixes: 5ea6b1702781 ("drm/panel: Add 
> > prepare_prev_first
> > flag to drm_panel").
> 
>  This kind of migration should be done *before* any possible
>  regression, not the other way round.
> 
>  If all panels sending commands from prepare() should have the
>  prepare_prev_first flag, then it should be first, check for
>  regressions then continue.
> 
>  
> 
> >>
> >> I understand, but this patch doesn't qualify as a fix for
> >> 9e15123eca79 and is too late to be merged in drm-misc-next for
> >> v6.6, and since 9e15123eca79 actually breaks some support it
> >> should be reverted (+ deps) since we are late in the rc cycles.
> >
> > If we go this way, we can never reapply these patches. There 
> 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-29 Thread Abhinav Kumar




On 8/29/2023 11:51 AM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 20:22, Abhinav Kumar  wrote:




On 8/29/2023 9:43 AM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 19:37, Abhinav Kumar  wrote:




On 8/29/2023 2:26 AM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 12:22,  wrote:


On 28/08/2023 19:07, Abhinav Kumar wrote:

Hi Neil

Sorry I didnt respond earlier on this thread.

On 8/28/2023 1:49 AM, neil.armstr...@linaro.org wrote:

Hi Jessica,

On 25/08/2023 20:37, Jessica Zhang wrote:



On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:

Hi Maxime,

On 21/08/2023 10:17, Maxime Ripard wrote:

Hi,

On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org wrote:

On 17/08/2023 20:35, Dmitry Baryshkov wrote:

On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:

Sending HS commands will always work on any controller, it's all
about LP commands. The Samsung panels you listed only send HS
commands so they can use prepare_prev_first and work on any
controllers.


I think there is some misunderstanding there, supported by the
description of the flag.

If I remember correctly, some hosts (sunxi) can not send DCS
commands after enabling video stream and switching to HS mode, see
[1]. Thus, as you know, most of the drivers have all DSI panel setup
commands in drm_panel_funcs::prepare() /
drm_bridge_funcs::pre_enable() callbacks, not paying attention
whether these commands are to be sent in LP or in HS mode.

Previously DSI source drivers could power on the DSI link either in
mode_set() or in pre_enable() callbacks, with mode_set() being the
hack to make panel/bridge drivers to be able to send commands from
their prepare() / pre_enable() callbacks.

With the prev_first flags being introduced, we have established that
DSI link should be enabled in DSI host's pre_enable() callback and
switched to HS mode (be it command or video) in the enable()
callback.

So far so good.


It seems coherent, I would like first to have a state of all DSI host
drivers and make this would actually work first before adding the
prev_first flag to all the required panels.


This is definitely what we should do in an ideal world, but at least for
sunxi there's no easy way for it at the moment. There's no documentation
for it and the driver provided doesn't allow this to happen.

Note that I'm not trying to discourage you or something here, I'm simply
pointing out that this will be something that we will have to take into
account. And it's possible that other drivers are in a similar
situation.


Unfortunately this change is not fully backwards-compatible. This
requires that all DSI panels sending commands from prepare() should
have the prepare_prev_first flag. In some sense, all such patches
might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
flag to drm_panel").


This kind of migration should be done *before* any possible
regression, not the other way round.

If all panels sending commands from prepare() should have the
prepare_prev_first flag, then it should be first, check for
regressions then continue.





I understand, but this patch doesn't qualify as a fix for
9e15123eca79 and is too late to be merged in drm-misc-next for
v6.6, and since 9e15123eca79 actually breaks some support it
should be reverted (+ deps) since we are late in the rc cycles.


If we go this way, we can never reapply these patches. There will be
no guarantee that all panel drivers are completely converted. We
already have a story without an observable end -
DRM_BRIDGE_ATTACH_NO_CONNECTOR.


I don't understand this point, who would block re-applying the patches ?

The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
Linux version and went smoothly because we reverted regressing patches
and restarted when needed, I don't understand why we can't do this
here aswell.


I'd consider that the DSI driver is correct here and it is about the
panel drivers that require fixes patches. If you care about the
particular Fixes tag, I have provided one several lines above.


Unfortunately it should be done in the other way round, prepare for
migration, then migrate,

I mean if it's a required migration, then it should be done and I'll
support it from both bridge and panel PoV.

So, first this patch has the wrong Fixes tag, and I would like a
better explanation on the commit message in any case. Then I would
like to have an ack from some drm-misc maintainers before applying it
because it fixes a patch that was sent via the msm tree thus per the
drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.


Sorry, it's not clear to me what you'd like our feedback on exactly?


So let me resume the situation:

- pre_enable_prev_first was introduced in [1]
- some panels made use of pre_enable_prev_first
- Visionox VTDR6130 was enabled on SM8550 systems and works on v6.5 kernels and 
before
- patch [2] was introduced on MSM DRM tree, breaking VTDR6130 on SM8550 systems 
(and probably other Video mode panels on 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-29 Thread Dmitry Baryshkov
On Tue, 29 Aug 2023 at 20:22, Abhinav Kumar  wrote:
>
>
>
> On 8/29/2023 9:43 AM, Dmitry Baryshkov wrote:
> > On Tue, 29 Aug 2023 at 19:37, Abhinav Kumar  
> > wrote:
> >>
> >>
> >>
> >> On 8/29/2023 2:26 AM, Dmitry Baryshkov wrote:
> >>> On Tue, 29 Aug 2023 at 12:22,  wrote:
> 
>  On 28/08/2023 19:07, Abhinav Kumar wrote:
> > Hi Neil
> >
> > Sorry I didnt respond earlier on this thread.
> >
> > On 8/28/2023 1:49 AM, neil.armstr...@linaro.org wrote:
> >> Hi Jessica,
> >>
> >> On 25/08/2023 20:37, Jessica Zhang wrote:
> >>>
> >>>
> >>> On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:
>  Hi Maxime,
> 
>  On 21/08/2023 10:17, Maxime Ripard wrote:
> > Hi,
> >
> > On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org 
> > wrote:
> >> On 17/08/2023 20:35, Dmitry Baryshkov wrote:
> >>> On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:
>  Sending HS commands will always work on any controller, it's all
>  about LP commands. The Samsung panels you listed only send HS
>  commands so they can use prepare_prev_first and work on any
>  controllers.
> >>>
> >>> I think there is some misunderstanding there, supported by the
> >>> description of the flag.
> >>>
> >>> If I remember correctly, some hosts (sunxi) can not send DCS
> >>> commands after enabling video stream and switching to HS mode, see
> >>> [1]. Thus, as you know, most of the drivers have all DSI panel 
> >>> setup
> >>> commands in drm_panel_funcs::prepare() /
> >>> drm_bridge_funcs::pre_enable() callbacks, not paying attention
> >>> whether these commands are to be sent in LP or in HS mode.
> >>>
> >>> Previously DSI source drivers could power on the DSI link either 
> >>> in
> >>> mode_set() or in pre_enable() callbacks, with mode_set() being the
> >>> hack to make panel/bridge drivers to be able to send commands from
> >>> their prepare() / pre_enable() callbacks.
> >>>
> >>> With the prev_first flags being introduced, we have established 
> >>> that
> >>> DSI link should be enabled in DSI host's pre_enable() callback and
> >>> switched to HS mode (be it command or video) in the enable()
> >>> callback.
> >>>
> >>> So far so good.
> >>
> >> It seems coherent, I would like first to have a state of all DSI 
> >> host
> >> drivers and make this would actually work first before adding the
> >> prev_first flag to all the required panels.
> >
> > This is definitely what we should do in an ideal world, but at 
> > least for
> > sunxi there's no easy way for it at the moment. There's no 
> > documentation
> > for it and the driver provided doesn't allow this to happen.
> >
> > Note that I'm not trying to discourage you or something here, I'm 
> > simply
> > pointing out that this will be something that we will have to take 
> > into
> > account. And it's possible that other drivers are in a similar
> > situation.
> >
> >>> Unfortunately this change is not fully backwards-compatible. This
> >>> requires that all DSI panels sending commands from prepare() 
> >>> should
> >>> have the prepare_prev_first flag. In some sense, all such patches
> >>> might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
> >>> flag to drm_panel").
> >>
> >> This kind of migration should be done *before* any possible
> >> regression, not the other way round.
> >>
> >> If all panels sending commands from prepare() should have the
> >> prepare_prev_first flag, then it should be first, check for
> >> regressions then continue.
> >>
> >> 
> >>
> 
>  I understand, but this patch doesn't qualify as a fix for
>  9e15123eca79 and is too late to be merged in drm-misc-next for
>  v6.6, and since 9e15123eca79 actually breaks some support it
>  should be reverted (+ deps) since we are late in the rc cycles.
> >>>
> >>> If we go this way, we can never reapply these patches. There will 
> >>> be
> >>> no guarantee that all panel drivers are completely converted. We
> >>> already have a story without an observable end -
> >>> DRM_BRIDGE_ATTACH_NO_CONNECTOR.
> >>
> >> I don't understand this point, who would block re-applying the 
> >> patches ?
> >>
> >> The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over 
> >> multiple
> >> Linux version and went 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-29 Thread Abhinav Kumar




On 8/29/2023 9:43 AM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 19:37, Abhinav Kumar  wrote:




On 8/29/2023 2:26 AM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 12:22,  wrote:


On 28/08/2023 19:07, Abhinav Kumar wrote:

Hi Neil

Sorry I didnt respond earlier on this thread.

On 8/28/2023 1:49 AM, neil.armstr...@linaro.org wrote:

Hi Jessica,

On 25/08/2023 20:37, Jessica Zhang wrote:



On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:

Hi Maxime,

On 21/08/2023 10:17, Maxime Ripard wrote:

Hi,

On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org wrote:

On 17/08/2023 20:35, Dmitry Baryshkov wrote:

On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:

Sending HS commands will always work on any controller, it's all
about LP commands. The Samsung panels you listed only send HS
commands so they can use prepare_prev_first and work on any
controllers.


I think there is some misunderstanding there, supported by the
description of the flag.

If I remember correctly, some hosts (sunxi) can not send DCS
commands after enabling video stream and switching to HS mode, see
[1]. Thus, as you know, most of the drivers have all DSI panel setup
commands in drm_panel_funcs::prepare() /
drm_bridge_funcs::pre_enable() callbacks, not paying attention
whether these commands are to be sent in LP or in HS mode.

Previously DSI source drivers could power on the DSI link either in
mode_set() or in pre_enable() callbacks, with mode_set() being the
hack to make panel/bridge drivers to be able to send commands from
their prepare() / pre_enable() callbacks.

With the prev_first flags being introduced, we have established that
DSI link should be enabled in DSI host's pre_enable() callback and
switched to HS mode (be it command or video) in the enable()
callback.

So far so good.


It seems coherent, I would like first to have a state of all DSI host
drivers and make this would actually work first before adding the
prev_first flag to all the required panels.


This is definitely what we should do in an ideal world, but at least for
sunxi there's no easy way for it at the moment. There's no documentation
for it and the driver provided doesn't allow this to happen.

Note that I'm not trying to discourage you or something here, I'm simply
pointing out that this will be something that we will have to take into
account. And it's possible that other drivers are in a similar
situation.


Unfortunately this change is not fully backwards-compatible. This
requires that all DSI panels sending commands from prepare() should
have the prepare_prev_first flag. In some sense, all such patches
might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
flag to drm_panel").


This kind of migration should be done *before* any possible
regression, not the other way round.

If all panels sending commands from prepare() should have the
prepare_prev_first flag, then it should be first, check for
regressions then continue.





I understand, but this patch doesn't qualify as a fix for
9e15123eca79 and is too late to be merged in drm-misc-next for
v6.6, and since 9e15123eca79 actually breaks some support it
should be reverted (+ deps) since we are late in the rc cycles.


If we go this way, we can never reapply these patches. There will be
no guarantee that all panel drivers are completely converted. We
already have a story without an observable end -
DRM_BRIDGE_ATTACH_NO_CONNECTOR.


I don't understand this point, who would block re-applying the patches ?

The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
Linux version and went smoothly because we reverted regressing patches
and restarted when needed, I don't understand why we can't do this
here aswell.


I'd consider that the DSI driver is correct here and it is about the
panel drivers that require fixes patches. If you care about the
particular Fixes tag, I have provided one several lines above.


Unfortunately it should be done in the other way round, prepare for
migration, then migrate,

I mean if it's a required migration, then it should be done and I'll
support it from both bridge and panel PoV.

So, first this patch has the wrong Fixes tag, and I would like a
better explanation on the commit message in any case. Then I would
like to have an ack from some drm-misc maintainers before applying it
because it fixes a patch that was sent via the msm tree thus per the
drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.


Sorry, it's not clear to me what you'd like our feedback on exactly?


So let me resume the situation:

- pre_enable_prev_first was introduced in [1]
- some panels made use of pre_enable_prev_first
- Visionox VTDR6130 was enabled on SM8550 systems and works on v6.5 kernels and 
before
- patch [2] was introduced on MSM DRM tree, breaking VTDR6130 on SM8550 systems 
(and probably other Video mode panels on Qcom platforms)
- this fix was sent late, and is now too late to be merged via drm-misc-next


Hi Neil and 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-29 Thread Dmitry Baryshkov
On Tue, 29 Aug 2023 at 19:37, Abhinav Kumar  wrote:
>
>
>
> On 8/29/2023 2:26 AM, Dmitry Baryshkov wrote:
> > On Tue, 29 Aug 2023 at 12:22,  wrote:
> >>
> >> On 28/08/2023 19:07, Abhinav Kumar wrote:
> >>> Hi Neil
> >>>
> >>> Sorry I didnt respond earlier on this thread.
> >>>
> >>> On 8/28/2023 1:49 AM, neil.armstr...@linaro.org wrote:
>  Hi Jessica,
> 
>  On 25/08/2023 20:37, Jessica Zhang wrote:
> >
> >
> > On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:
> >> Hi Maxime,
> >>
> >> On 21/08/2023 10:17, Maxime Ripard wrote:
> >>> Hi,
> >>>
> >>> On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org 
> >>> wrote:
>  On 17/08/2023 20:35, Dmitry Baryshkov wrote:
> > On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:
> >> Sending HS commands will always work on any controller, it's all
> >> about LP commands. The Samsung panels you listed only send HS
> >> commands so they can use prepare_prev_first and work on any
> >> controllers.
> >
> > I think there is some misunderstanding there, supported by the
> > description of the flag.
> >
> > If I remember correctly, some hosts (sunxi) can not send DCS
> > commands after enabling video stream and switching to HS mode, see
> > [1]. Thus, as you know, most of the drivers have all DSI panel setup
> > commands in drm_panel_funcs::prepare() /
> > drm_bridge_funcs::pre_enable() callbacks, not paying attention
> > whether these commands are to be sent in LP or in HS mode.
> >
> > Previously DSI source drivers could power on the DSI link either in
> > mode_set() or in pre_enable() callbacks, with mode_set() being the
> > hack to make panel/bridge drivers to be able to send commands from
> > their prepare() / pre_enable() callbacks.
> >
> > With the prev_first flags being introduced, we have established that
> > DSI link should be enabled in DSI host's pre_enable() callback and
> > switched to HS mode (be it command or video) in the enable()
> > callback.
> >
> > So far so good.
> 
>  It seems coherent, I would like first to have a state of all DSI host
>  drivers and make this would actually work first before adding the
>  prev_first flag to all the required panels.
> >>>
> >>> This is definitely what we should do in an ideal world, but at least 
> >>> for
> >>> sunxi there's no easy way for it at the moment. There's no 
> >>> documentation
> >>> for it and the driver provided doesn't allow this to happen.
> >>>
> >>> Note that I'm not trying to discourage you or something here, I'm 
> >>> simply
> >>> pointing out that this will be something that we will have to take 
> >>> into
> >>> account. And it's possible that other drivers are in a similar
> >>> situation.
> >>>
> > Unfortunately this change is not fully backwards-compatible. This
> > requires that all DSI panels sending commands from prepare() should
> > have the prepare_prev_first flag. In some sense, all such patches
> > might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
> > flag to drm_panel").
> 
>  This kind of migration should be done *before* any possible
>  regression, not the other way round.
> 
>  If all panels sending commands from prepare() should have the
>  prepare_prev_first flag, then it should be first, check for
>  regressions then continue.
> 
>  
> 
> >>
> >> I understand, but this patch doesn't qualify as a fix for
> >> 9e15123eca79 and is too late to be merged in drm-misc-next for
> >> v6.6, and since 9e15123eca79 actually breaks some support it
> >> should be reverted (+ deps) since we are late in the rc cycles.
> >
> > If we go this way, we can never reapply these patches. There will be
> > no guarantee that all panel drivers are completely converted. We
> > already have a story without an observable end -
> > DRM_BRIDGE_ATTACH_NO_CONNECTOR.
> 
>  I don't understand this point, who would block re-applying the 
>  patches ?
> 
>  The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over 
>  multiple
>  Linux version and went smoothly because we reverted regressing 
>  patches
>  and restarted when needed, I don't understand why we can't do this
>  here aswell.
> 
> > I'd consider that the DSI driver is correct here and it is about the
> > panel drivers that require fixes patches. If you care about the
> > particular Fixes tag, I have provided one several lines above.
> 
> 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-29 Thread Abhinav Kumar




On 8/29/2023 2:26 AM, Dmitry Baryshkov wrote:

On Tue, 29 Aug 2023 at 12:22,  wrote:


On 28/08/2023 19:07, Abhinav Kumar wrote:

Hi Neil

Sorry I didnt respond earlier on this thread.

On 8/28/2023 1:49 AM, neil.armstr...@linaro.org wrote:

Hi Jessica,

On 25/08/2023 20:37, Jessica Zhang wrote:



On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:

Hi Maxime,

On 21/08/2023 10:17, Maxime Ripard wrote:

Hi,

On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org wrote:

On 17/08/2023 20:35, Dmitry Baryshkov wrote:

On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:

Sending HS commands will always work on any controller, it's all
about LP commands. The Samsung panels you listed only send HS
commands so they can use prepare_prev_first and work on any
controllers.


I think there is some misunderstanding there, supported by the
description of the flag.

If I remember correctly, some hosts (sunxi) can not send DCS
commands after enabling video stream and switching to HS mode, see
[1]. Thus, as you know, most of the drivers have all DSI panel setup
commands in drm_panel_funcs::prepare() /
drm_bridge_funcs::pre_enable() callbacks, not paying attention
whether these commands are to be sent in LP or in HS mode.

Previously DSI source drivers could power on the DSI link either in
mode_set() or in pre_enable() callbacks, with mode_set() being the
hack to make panel/bridge drivers to be able to send commands from
their prepare() / pre_enable() callbacks.

With the prev_first flags being introduced, we have established that
DSI link should be enabled in DSI host's pre_enable() callback and
switched to HS mode (be it command or video) in the enable()
callback.

So far so good.


It seems coherent, I would like first to have a state of all DSI host
drivers and make this would actually work first before adding the
prev_first flag to all the required panels.


This is definitely what we should do in an ideal world, but at least for
sunxi there's no easy way for it at the moment. There's no documentation
for it and the driver provided doesn't allow this to happen.

Note that I'm not trying to discourage you or something here, I'm simply
pointing out that this will be something that we will have to take into
account. And it's possible that other drivers are in a similar
situation.


Unfortunately this change is not fully backwards-compatible. This
requires that all DSI panels sending commands from prepare() should
have the prepare_prev_first flag. In some sense, all such patches
might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
flag to drm_panel").


This kind of migration should be done *before* any possible
regression, not the other way round.

If all panels sending commands from prepare() should have the
prepare_prev_first flag, then it should be first, check for
regressions then continue.





I understand, but this patch doesn't qualify as a fix for
9e15123eca79 and is too late to be merged in drm-misc-next for
v6.6, and since 9e15123eca79 actually breaks some support it
should be reverted (+ deps) since we are late in the rc cycles.


If we go this way, we can never reapply these patches. There will be
no guarantee that all panel drivers are completely converted. We
already have a story without an observable end -
DRM_BRIDGE_ATTACH_NO_CONNECTOR.


I don't understand this point, who would block re-applying the patches ?

The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
Linux version and went smoothly because we reverted regressing patches
and restarted when needed, I don't understand why we can't do this
here aswell.


I'd consider that the DSI driver is correct here and it is about the
panel drivers that require fixes patches. If you care about the
particular Fixes tag, I have provided one several lines above.


Unfortunately it should be done in the other way round, prepare for
migration, then migrate,

I mean if it's a required migration, then it should be done and I'll
support it from both bridge and panel PoV.

So, first this patch has the wrong Fixes tag, and I would like a
better explanation on the commit message in any case. Then I would
like to have an ack from some drm-misc maintainers before applying it
because it fixes a patch that was sent via the msm tree thus per the
drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.


Sorry, it's not clear to me what you'd like our feedback on exactly?


So let me resume the situation:

- pre_enable_prev_first was introduced in [1]
- some panels made use of pre_enable_prev_first
- Visionox VTDR6130 was enabled on SM8550 systems and works on v6.5 kernels and 
before
- patch [2] was introduced on MSM DRM tree, breaking VTDR6130 on SM8550 systems 
(and probably other Video mode panels on Qcom platforms)
- this fix was sent late, and is now too late to be merged via drm-misc-next


Hi Neil and Maxime,

I agree with Neil that 9e15123eca79 was the commit that introduced the issue 
(since it 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-29 Thread Dave Stevenson
Hi Neil

On Mon, 28 Aug 2023 at 09:49,  wrote:
>
> Hi Jessica,
>
> On 25/08/2023 20:37, Jessica Zhang wrote:
> >
> >
> > On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:
> >> Hi Maxime,
> >>
> >> On 21/08/2023 10:17, Maxime Ripard wrote:
> >>> Hi,
> >>>
> >>> On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org wrote:
>  On 17/08/2023 20:35, Dmitry Baryshkov wrote:
> > On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:
> >> Sending HS commands will always work on any controller, it's all
> >> about LP commands. The Samsung panels you listed only send HS
> >> commands so they can use prepare_prev_first and work on any
> >> controllers.
> >
> > I think there is some misunderstanding there, supported by the
> > description of the flag.
> >
> > If I remember correctly, some hosts (sunxi) can not send DCS
> > commands after enabling video stream and switching to HS mode, see
> > [1]. Thus, as you know, most of the drivers have all DSI panel setup
> > commands in drm_panel_funcs::prepare() /
> > drm_bridge_funcs::pre_enable() callbacks, not paying attention
> > whether these commands are to be sent in LP or in HS mode.
> >
> > Previously DSI source drivers could power on the DSI link either in
> > mode_set() or in pre_enable() callbacks, with mode_set() being the
> > hack to make panel/bridge drivers to be able to send commands from
> > their prepare() / pre_enable() callbacks.
> >
> > With the prev_first flags being introduced, we have established that
> > DSI link should be enabled in DSI host's pre_enable() callback and
> > switched to HS mode (be it command or video) in the enable()
> > callback.
> >
> > So far so good.
> 
>  It seems coherent, I would like first to have a state of all DSI host
>  drivers and make this would actually work first before adding the
>  prev_first flag to all the required panels.
> >>>
> >>> This is definitely what we should do in an ideal world, but at least for
> >>> sunxi there's no easy way for it at the moment. There's no documentation
> >>> for it and the driver provided doesn't allow this to happen.
> >>>
> >>> Note that I'm not trying to discourage you or something here, I'm simply
> >>> pointing out that this will be something that we will have to take into
> >>> account. And it's possible that other drivers are in a similar
> >>> situation.
> >>>
> > Unfortunately this change is not fully backwards-compatible. This
> > requires that all DSI panels sending commands from prepare() should
> > have the prepare_prev_first flag. In some sense, all such patches
> > might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
> > flag to drm_panel").
> 
>  This kind of migration should be done *before* any possible
>  regression, not the other way round.
> 
>  If all panels sending commands from prepare() should have the
>  prepare_prev_first flag, then it should be first, check for
>  regressions then continue.
> 
>  
> 
> >>
> >> I understand, but this patch doesn't qualify as a fix for
> >> 9e15123eca79 and is too late to be merged in drm-misc-next for
> >> v6.6, and since 9e15123eca79 actually breaks some support it
> >> should be reverted (+ deps) since we are late in the rc cycles.
> >
> > If we go this way, we can never reapply these patches. There will be
> > no guarantee that all panel drivers are completely converted. We
> > already have a story without an observable end -
> > DRM_BRIDGE_ATTACH_NO_CONNECTOR.
> 
>  I don't understand this point, who would block re-applying the patches ?
> 
>  The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
>  Linux version and went smoothly because we reverted regressing patches
>  and restarted when needed, I don't understand why we can't do this
>  here aswell.
> 
> > I'd consider that the DSI driver is correct here and it is about the
> > panel drivers that require fixes patches. If you care about the
> > particular Fixes tag, I have provided one several lines above.
> 
>  Unfortunately it should be done in the other way round, prepare for
>  migration, then migrate,
> 
>  I mean if it's a required migration, then it should be done and I'll
>  support it from both bridge and panel PoV.
> 
>  So, first this patch has the wrong Fixes tag, and I would like a
>  better explanation on the commit message in any case. Then I would
>  like to have an ack from some drm-misc maintainers before applying it
>  because it fixes a patch that was sent via the msm tree thus per the
>  drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.
> >>>
> >>> Sorry, it's not clear to me what you'd like our feedback on exactly?
> >>
> >> So let me resume the situation:
> >>
> >> 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-29 Thread Dmitry Baryshkov
On Tue, 29 Aug 2023 at 12:22,  wrote:
>
> On 28/08/2023 19:07, Abhinav Kumar wrote:
> > Hi Neil
> >
> > Sorry I didnt respond earlier on this thread.
> >
> > On 8/28/2023 1:49 AM, neil.armstr...@linaro.org wrote:
> >> Hi Jessica,
> >>
> >> On 25/08/2023 20:37, Jessica Zhang wrote:
> >>>
> >>>
> >>> On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:
>  Hi Maxime,
> 
>  On 21/08/2023 10:17, Maxime Ripard wrote:
> > Hi,
> >
> > On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org 
> > wrote:
> >> On 17/08/2023 20:35, Dmitry Baryshkov wrote:
> >>> On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:
>  Sending HS commands will always work on any controller, it's all
>  about LP commands. The Samsung panels you listed only send HS
>  commands so they can use prepare_prev_first and work on any
>  controllers.
> >>>
> >>> I think there is some misunderstanding there, supported by the
> >>> description of the flag.
> >>>
> >>> If I remember correctly, some hosts (sunxi) can not send DCS
> >>> commands after enabling video stream and switching to HS mode, see
> >>> [1]. Thus, as you know, most of the drivers have all DSI panel setup
> >>> commands in drm_panel_funcs::prepare() /
> >>> drm_bridge_funcs::pre_enable() callbacks, not paying attention
> >>> whether these commands are to be sent in LP or in HS mode.
> >>>
> >>> Previously DSI source drivers could power on the DSI link either in
> >>> mode_set() or in pre_enable() callbacks, with mode_set() being the
> >>> hack to make panel/bridge drivers to be able to send commands from
> >>> their prepare() / pre_enable() callbacks.
> >>>
> >>> With the prev_first flags being introduced, we have established that
> >>> DSI link should be enabled in DSI host's pre_enable() callback and
> >>> switched to HS mode (be it command or video) in the enable()
> >>> callback.
> >>>
> >>> So far so good.
> >>
> >> It seems coherent, I would like first to have a state of all DSI host
> >> drivers and make this would actually work first before adding the
> >> prev_first flag to all the required panels.
> >
> > This is definitely what we should do in an ideal world, but at least for
> > sunxi there's no easy way for it at the moment. There's no documentation
> > for it and the driver provided doesn't allow this to happen.
> >
> > Note that I'm not trying to discourage you or something here, I'm simply
> > pointing out that this will be something that we will have to take into
> > account. And it's possible that other drivers are in a similar
> > situation.
> >
> >>> Unfortunately this change is not fully backwards-compatible. This
> >>> requires that all DSI panels sending commands from prepare() should
> >>> have the prepare_prev_first flag. In some sense, all such patches
> >>> might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
> >>> flag to drm_panel").
> >>
> >> This kind of migration should be done *before* any possible
> >> regression, not the other way round.
> >>
> >> If all panels sending commands from prepare() should have the
> >> prepare_prev_first flag, then it should be first, check for
> >> regressions then continue.
> >>
> >> 
> >>
> 
>  I understand, but this patch doesn't qualify as a fix for
>  9e15123eca79 and is too late to be merged in drm-misc-next for
>  v6.6, and since 9e15123eca79 actually breaks some support it
>  should be reverted (+ deps) since we are late in the rc cycles.
> >>>
> >>> If we go this way, we can never reapply these patches. There will be
> >>> no guarantee that all panel drivers are completely converted. We
> >>> already have a story without an observable end -
> >>> DRM_BRIDGE_ATTACH_NO_CONNECTOR.
> >>
> >> I don't understand this point, who would block re-applying the patches 
> >> ?
> >>
> >> The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
> >> Linux version and went smoothly because we reverted regressing patches
> >> and restarted when needed, I don't understand why we can't do this
> >> here aswell.
> >>
> >>> I'd consider that the DSI driver is correct here and it is about the
> >>> panel drivers that require fixes patches. If you care about the
> >>> particular Fixes tag, I have provided one several lines above.
> >>
> >> Unfortunately it should be done in the other way round, prepare for
> >> migration, then migrate,
> >>
> >> I mean if it's a required migration, then it should be done and I'll
> >> support it from both bridge and panel PoV.
> >>
> >> So, first this patch has the wrong Fixes tag, and I would like a
> >> better explanation on the commit message in 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-29 Thread neil . armstrong

On 28/08/2023 19:07, Abhinav Kumar wrote:

Hi Neil

Sorry I didnt respond earlier on this thread.

On 8/28/2023 1:49 AM, neil.armstr...@linaro.org wrote:

Hi Jessica,

On 25/08/2023 20:37, Jessica Zhang wrote:



On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:

Hi Maxime,

On 21/08/2023 10:17, Maxime Ripard wrote:

Hi,

On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org wrote:

On 17/08/2023 20:35, Dmitry Baryshkov wrote:

On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:

Sending HS commands will always work on any controller, it's all
about LP commands. The Samsung panels you listed only send HS
commands so they can use prepare_prev_first and work on any
controllers.


I think there is some misunderstanding there, supported by the
description of the flag.

If I remember correctly, some hosts (sunxi) can not send DCS
commands after enabling video stream and switching to HS mode, see
[1]. Thus, as you know, most of the drivers have all DSI panel setup
commands in drm_panel_funcs::prepare() /
drm_bridge_funcs::pre_enable() callbacks, not paying attention
whether these commands are to be sent in LP or in HS mode.

Previously DSI source drivers could power on the DSI link either in
mode_set() or in pre_enable() callbacks, with mode_set() being the
hack to make panel/bridge drivers to be able to send commands from
their prepare() / pre_enable() callbacks.

With the prev_first flags being introduced, we have established that
DSI link should be enabled in DSI host's pre_enable() callback and
switched to HS mode (be it command or video) in the enable()
callback.

So far so good.


It seems coherent, I would like first to have a state of all DSI host
drivers and make this would actually work first before adding the
prev_first flag to all the required panels.


This is definitely what we should do in an ideal world, but at least for
sunxi there's no easy way for it at the moment. There's no documentation
for it and the driver provided doesn't allow this to happen.

Note that I'm not trying to discourage you or something here, I'm simply
pointing out that this will be something that we will have to take into
account. And it's possible that other drivers are in a similar
situation.


Unfortunately this change is not fully backwards-compatible. This
requires that all DSI panels sending commands from prepare() should
have the prepare_prev_first flag. In some sense, all such patches
might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
flag to drm_panel").


This kind of migration should be done *before* any possible
regression, not the other way round.

If all panels sending commands from prepare() should have the
prepare_prev_first flag, then it should be first, check for
regressions then continue.





I understand, but this patch doesn't qualify as a fix for
9e15123eca79 and is too late to be merged in drm-misc-next for
v6.6, and since 9e15123eca79 actually breaks some support it
should be reverted (+ deps) since we are late in the rc cycles.


If we go this way, we can never reapply these patches. There will be
no guarantee that all panel drivers are completely converted. We
already have a story without an observable end -
DRM_BRIDGE_ATTACH_NO_CONNECTOR.


I don't understand this point, who would block re-applying the patches ?

The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
Linux version and went smoothly because we reverted regressing patches
and restarted when needed, I don't understand why we can't do this
here aswell.


I'd consider that the DSI driver is correct here and it is about the
panel drivers that require fixes patches. If you care about the
particular Fixes tag, I have provided one several lines above.


Unfortunately it should be done in the other way round, prepare for
migration, then migrate,

I mean if it's a required migration, then it should be done and I'll
support it from both bridge and panel PoV.

So, first this patch has the wrong Fixes tag, and I would like a
better explanation on the commit message in any case. Then I would
like to have an ack from some drm-misc maintainers before applying it
because it fixes a patch that was sent via the msm tree thus per the
drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.


Sorry, it's not clear to me what you'd like our feedback on exactly?


So let me resume the situation:

- pre_enable_prev_first was introduced in [1]
- some panels made use of pre_enable_prev_first
- Visionox VTDR6130 was enabled on SM8550 systems and works on v6.5 kernels and 
before
- patch [2] was introduced on MSM DRM tree, breaking VTDR6130 on SM8550 systems 
(and probably other Video mode panels on Qcom platforms)
- this fix was sent late, and is now too late to be merged via drm-misc-next


Hi Neil and Maxime,

I agree with Neil that 9e15123eca79 was the commit that introduced the issue 
(since it changed the MSM DSI host behavior).

However, I'm not too keen on simply reverting that patch 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-28 Thread Abhinav Kumar

Hi Neil

Sorry I didnt respond earlier on this thread.

On 8/28/2023 1:49 AM, neil.armstr...@linaro.org wrote:

Hi Jessica,

On 25/08/2023 20:37, Jessica Zhang wrote:



On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:

Hi Maxime,

On 21/08/2023 10:17, Maxime Ripard wrote:

Hi,

On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org 
wrote:

On 17/08/2023 20:35, Dmitry Baryshkov wrote:

On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:

Sending HS commands will always work on any controller, it's all
about LP commands. The Samsung panels you listed only send HS
commands so they can use prepare_prev_first and work on any
controllers.


I think there is some misunderstanding there, supported by the
description of the flag.

If I remember correctly, some hosts (sunxi) can not send DCS
commands after enabling video stream and switching to HS mode, see
[1]. Thus, as you know, most of the drivers have all DSI panel setup
commands in drm_panel_funcs::prepare() /
drm_bridge_funcs::pre_enable() callbacks, not paying attention
whether these commands are to be sent in LP or in HS mode.

Previously DSI source drivers could power on the DSI link either in
mode_set() or in pre_enable() callbacks, with mode_set() being the
hack to make panel/bridge drivers to be able to send commands from
their prepare() / pre_enable() callbacks.

With the prev_first flags being introduced, we have established that
DSI link should be enabled in DSI host's pre_enable() callback and
switched to HS mode (be it command or video) in the enable()
callback.

So far so good.


It seems coherent, I would like first to have a state of all DSI host
drivers and make this would actually work first before adding the
prev_first flag to all the required panels.


This is definitely what we should do in an ideal world, but at least 
for
sunxi there's no easy way for it at the moment. There's no 
documentation

for it and the driver provided doesn't allow this to happen.

Note that I'm not trying to discourage you or something here, I'm 
simply

pointing out that this will be something that we will have to take into
account. And it's possible that other drivers are in a similar
situation.


Unfortunately this change is not fully backwards-compatible. This
requires that all DSI panels sending commands from prepare() should
have the prepare_prev_first flag. In some sense, all such patches
might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
flag to drm_panel").


This kind of migration should be done *before* any possible
regression, not the other way round.

If all panels sending commands from prepare() should have the
prepare_prev_first flag, then it should be first, check for
regressions then continue.





I understand, but this patch doesn't qualify as a fix for
9e15123eca79 and is too late to be merged in drm-misc-next for
v6.6, and since 9e15123eca79 actually breaks some support it
should be reverted (+ deps) since we are late in the rc cycles.


If we go this way, we can never reapply these patches. There will be
no guarantee that all panel drivers are completely converted. We
already have a story without an observable end -
DRM_BRIDGE_ATTACH_NO_CONNECTOR.


I don't understand this point, who would block re-applying the 
patches ?


The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
Linux version and went smoothly because we reverted regressing patches
and restarted when needed, I don't understand why we can't do this
here aswell.


I'd consider that the DSI driver is correct here and it is about the
panel drivers that require fixes patches. If you care about the
particular Fixes tag, I have provided one several lines above.


Unfortunately it should be done in the other way round, prepare for
migration, then migrate,

I mean if it's a required migration, then it should be done and I'll
support it from both bridge and panel PoV.

So, first this patch has the wrong Fixes tag, and I would like a
better explanation on the commit message in any case. Then I would
like to have an ack from some drm-misc maintainers before applying it
because it fixes a patch that was sent via the msm tree thus per the
drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.


Sorry, it's not clear to me what you'd like our feedback on exactly?


So let me resume the situation:

- pre_enable_prev_first was introduced in [1]
- some panels made use of pre_enable_prev_first
- Visionox VTDR6130 was enabled on SM8550 systems and works on v6.5 
kernels and before
- patch [2] was introduced on MSM DRM tree, breaking VTDR6130 on 
SM8550 systems (and probably other Video mode panels on Qcom platforms)
- this fix was sent late, and is now too late to be merged via 
drm-misc-next


Hi Neil and Maxime,

I agree with Neil that 9e15123eca79 was the commit that introduced the 
issue (since it changed the MSM DSI host behavior).


However, I'm not too keen on simply reverting that patch because

1) it's not wrong to have 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-28 Thread neil . armstrong

Hi Jessica,

On 25/08/2023 20:37, Jessica Zhang wrote:



On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:

Hi Maxime,

On 21/08/2023 10:17, Maxime Ripard wrote:

Hi,

On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org wrote:

On 17/08/2023 20:35, Dmitry Baryshkov wrote:

On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:

Sending HS commands will always work on any controller, it's all
about LP commands. The Samsung panels you listed only send HS
commands so they can use prepare_prev_first and work on any
controllers.


I think there is some misunderstanding there, supported by the
description of the flag.

If I remember correctly, some hosts (sunxi) can not send DCS
commands after enabling video stream and switching to HS mode, see
[1]. Thus, as you know, most of the drivers have all DSI panel setup
commands in drm_panel_funcs::prepare() /
drm_bridge_funcs::pre_enable() callbacks, not paying attention
whether these commands are to be sent in LP or in HS mode.

Previously DSI source drivers could power on the DSI link either in
mode_set() or in pre_enable() callbacks, with mode_set() being the
hack to make panel/bridge drivers to be able to send commands from
their prepare() / pre_enable() callbacks.

With the prev_first flags being introduced, we have established that
DSI link should be enabled in DSI host's pre_enable() callback and
switched to HS mode (be it command or video) in the enable()
callback.

So far so good.


It seems coherent, I would like first to have a state of all DSI host
drivers and make this would actually work first before adding the
prev_first flag to all the required panels.


This is definitely what we should do in an ideal world, but at least for
sunxi there's no easy way for it at the moment. There's no documentation
for it and the driver provided doesn't allow this to happen.

Note that I'm not trying to discourage you or something here, I'm simply
pointing out that this will be something that we will have to take into
account. And it's possible that other drivers are in a similar
situation.


Unfortunately this change is not fully backwards-compatible. This
requires that all DSI panels sending commands from prepare() should
have the prepare_prev_first flag. In some sense, all such patches
might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
flag to drm_panel").


This kind of migration should be done *before* any possible
regression, not the other way round.

If all panels sending commands from prepare() should have the
prepare_prev_first flag, then it should be first, check for
regressions then continue.





I understand, but this patch doesn't qualify as a fix for
9e15123eca79 and is too late to be merged in drm-misc-next for
v6.6, and since 9e15123eca79 actually breaks some support it
should be reverted (+ deps) since we are late in the rc cycles.


If we go this way, we can never reapply these patches. There will be
no guarantee that all panel drivers are completely converted. We
already have a story without an observable end -
DRM_BRIDGE_ATTACH_NO_CONNECTOR.


I don't understand this point, who would block re-applying the patches ?

The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
Linux version and went smoothly because we reverted regressing patches
and restarted when needed, I don't understand why we can't do this
here aswell.


I'd consider that the DSI driver is correct here and it is about the
panel drivers that require fixes patches. If you care about the
particular Fixes tag, I have provided one several lines above.


Unfortunately it should be done in the other way round, prepare for
migration, then migrate,

I mean if it's a required migration, then it should be done and I'll
support it from both bridge and panel PoV.

So, first this patch has the wrong Fixes tag, and I would like a
better explanation on the commit message in any case. Then I would
like to have an ack from some drm-misc maintainers before applying it
because it fixes a patch that was sent via the msm tree thus per the
drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.


Sorry, it's not clear to me what you'd like our feedback on exactly?


So let me resume the situation:

- pre_enable_prev_first was introduced in [1]
- some panels made use of pre_enable_prev_first
- Visionox VTDR6130 was enabled on SM8550 systems and works on v6.5 kernels and 
before
- patch [2] was introduced on MSM DRM tree, breaking VTDR6130 on SM8550 systems 
(and probably other Video mode panels on Qcom platforms)
- this fix was sent late, and is now too late to be merged via drm-misc-next


Hi Neil and Maxime,

I agree with Neil that 9e15123eca79 was the commit that introduced the issue 
(since it changed the MSM DSI host behavior).

However, I'm not too keen on simply reverting that patch because

1) it's not wrong to have the dsi_power_on in pre_enable. Arguably, it actually 
makes more sense to power on DSI host in pre_enable than in 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-25 Thread Jessica Zhang




On 8/21/2023 3:01 AM, neil.armstr...@linaro.org wrote:

Hi Maxime,

On 21/08/2023 10:17, Maxime Ripard wrote:

Hi,

On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org 
wrote:

On 17/08/2023 20:35, Dmitry Baryshkov wrote:

On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:

Sending HS commands will always work on any controller, it's all
about LP commands. The Samsung panels you listed only send HS
commands so they can use prepare_prev_first and work on any
controllers.


I think there is some misunderstanding there, supported by the
description of the flag.

If I remember correctly, some hosts (sunxi) can not send DCS
commands after enabling video stream and switching to HS mode, see
[1]. Thus, as you know, most of the drivers have all DSI panel setup
commands in drm_panel_funcs::prepare() /
drm_bridge_funcs::pre_enable() callbacks, not paying attention
whether these commands are to be sent in LP or in HS mode.

Previously DSI source drivers could power on the DSI link either in
mode_set() or in pre_enable() callbacks, with mode_set() being the
hack to make panel/bridge drivers to be able to send commands from
their prepare() / pre_enable() callbacks.

With the prev_first flags being introduced, we have established that
DSI link should be enabled in DSI host's pre_enable() callback and
switched to HS mode (be it command or video) in the enable()
callback.

So far so good.


It seems coherent, I would like first to have a state of all DSI host
drivers and make this would actually work first before adding the
prev_first flag to all the required panels.


This is definitely what we should do in an ideal world, but at least for
sunxi there's no easy way for it at the moment. There's no documentation
for it and the driver provided doesn't allow this to happen.

Note that I'm not trying to discourage you or something here, I'm simply
pointing out that this will be something that we will have to take into
account. And it's possible that other drivers are in a similar
situation.


Unfortunately this change is not fully backwards-compatible. This
requires that all DSI panels sending commands from prepare() should
have the prepare_prev_first flag. In some sense, all such patches
might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
flag to drm_panel").


This kind of migration should be done *before* any possible
regression, not the other way round.

If all panels sending commands from prepare() should have the
prepare_prev_first flag, then it should be first, check for
regressions then continue.





I understand, but this patch doesn't qualify as a fix for
9e15123eca79 and is too late to be merged in drm-misc-next for
v6.6, and since 9e15123eca79 actually breaks some support it
should be reverted (+ deps) since we are late in the rc cycles.


If we go this way, we can never reapply these patches. There will be
no guarantee that all panel drivers are completely converted. We
already have a story without an observable end -
DRM_BRIDGE_ATTACH_NO_CONNECTOR.


I don't understand this point, who would block re-applying the patches ?

The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
Linux version and went smoothly because we reverted regressing patches
and restarted when needed, I don't understand why we can't do this
here aswell.


I'd consider that the DSI driver is correct here and it is about the
panel drivers that require fixes patches. If you care about the
particular Fixes tag, I have provided one several lines above.


Unfortunately it should be done in the other way round, prepare for
migration, then migrate,

I mean if it's a required migration, then it should be done and I'll
support it from both bridge and panel PoV.

So, first this patch has the wrong Fixes tag, and I would like a
better explanation on the commit message in any case. Then I would
like to have an ack from some drm-misc maintainers before applying it
because it fixes a patch that was sent via the msm tree thus per the
drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.


Sorry, it's not clear to me what you'd like our feedback on exactly?


So let me resume the situation:

- pre_enable_prev_first was introduced in [1]
- some panels made use of pre_enable_prev_first
- Visionox VTDR6130 was enabled on SM8550 systems and works on v6.5 
kernels and before
- patch [2] was introduced on MSM DRM tree, breaking VTDR6130 on SM8550 
systems (and probably other Video mode panels on Qcom platforms)
- this fix was sent late, and is now too late to be merged via 
drm-misc-next


Hi Neil and Maxime,

I agree with Neil that 9e15123eca79 was the commit that introduced the 
issue (since it changed the MSM DSI host behavior).


However, I'm not too keen on simply reverting that patch because

1) it's not wrong to have the dsi_power_on in pre_enable. Arguably, it 
actually makes more sense to power on DSI host in pre_enable than in 
modeset (since modeset is meant for setting the bridge 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-21 Thread Maxime Ripard
On Mon, Aug 21, 2023 at 12:01:19PM +0200, neil.armstr...@linaro.org wrote:
> Hi Maxime,
> 
> On 21/08/2023 10:17, Maxime Ripard wrote:
> > Hi,
> > 
> > On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org wrote:
> > > On 17/08/2023 20:35, Dmitry Baryshkov wrote:
> > > > On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:
> > > > > Sending HS commands will always work on any controller, it's all
> > > > > about LP commands. The Samsung panels you listed only send HS
> > > > > commands so they can use prepare_prev_first and work on any
> > > > > controllers.
> > > > 
> > > > I think there is some misunderstanding there, supported by the
> > > > description of the flag.
> > > > 
> > > > If I remember correctly, some hosts (sunxi) can not send DCS
> > > > commands after enabling video stream and switching to HS mode, see
> > > > [1]. Thus, as you know, most of the drivers have all DSI panel setup
> > > > commands in drm_panel_funcs::prepare() /
> > > > drm_bridge_funcs::pre_enable() callbacks, not paying attention
> > > > whether these commands are to be sent in LP or in HS mode.
> > > > 
> > > > Previously DSI source drivers could power on the DSI link either in
> > > > mode_set() or in pre_enable() callbacks, with mode_set() being the
> > > > hack to make panel/bridge drivers to be able to send commands from
> > > > their prepare() / pre_enable() callbacks.
> > > > 
> > > > With the prev_first flags being introduced, we have established that
> > > > DSI link should be enabled in DSI host's pre_enable() callback and
> > > > switched to HS mode (be it command or video) in the enable()
> > > > callback.
> > > > 
> > > > So far so good.
> > > 
> > > It seems coherent, I would like first to have a state of all DSI host
> > > drivers and make this would actually work first before adding the
> > > prev_first flag to all the required panels.
> > 
> > This is definitely what we should do in an ideal world, but at least for
> > sunxi there's no easy way for it at the moment. There's no documentation
> > for it and the driver provided doesn't allow this to happen.
> > 
> > Note that I'm not trying to discourage you or something here, I'm simply
> > pointing out that this will be something that we will have to take into
> > account. And it's possible that other drivers are in a similar
> > situation.
> > 
> > > > Unfortunately this change is not fully backwards-compatible. This
> > > > requires that all DSI panels sending commands from prepare() should
> > > > have the prepare_prev_first flag. In some sense, all such patches
> > > > might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
> > > > flag to drm_panel").
> > > 
> > > This kind of migration should be done *before* any possible
> > > regression, not the other way round.
> > > 
> > > If all panels sending commands from prepare() should have the
> > > prepare_prev_first flag, then it should be first, check for
> > > regressions then continue.
> > > 
> > > 
> > > 
> > > > > 
> > > > > I understand, but this patch doesn't qualify as a fix for
> > > > > 9e15123eca79 and is too late to be merged in drm-misc-next for
> > > > > v6.6, and since 9e15123eca79 actually breaks some support it
> > > > > should be reverted (+ deps) since we are late in the rc cycles.
> > > > 
> > > > If we go this way, we can never reapply these patches. There will be
> > > > no guarantee that all panel drivers are completely converted. We
> > > > already have a story without an observable end -
> > > > DRM_BRIDGE_ATTACH_NO_CONNECTOR.
> > > 
> > > I don't understand this point, who would block re-applying the patches ?
> > > 
> > > The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
> > > Linux version and went smoothly because we reverted regressing patches
> > > and restarted when needed, I don't understand why we can't do this
> > > here aswell.
> > > 
> > > > I'd consider that the DSI driver is correct here and it is about the
> > > > panel drivers that require fixes patches. If you care about the
> > > > particular Fixes tag, I have provided one several lines above.
> > > 
> > > Unfortunately it should be done in the other way round, prepare for
> > > migration, then migrate,
> > > 
> > > I mean if it's a required migration, then it should be done and I'll
> > > support it from both bridge and panel PoV.
> > > 
> > > So, first this patch has the wrong Fixes tag, and I would like a
> > > better explanation on the commit message in any case. Then I would
> > > like to have an ack from some drm-misc maintainers before applying it
> > > because it fixes a patch that was sent via the msm tree thus per the
> > > drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.
> > 
> > Sorry, it's not clear to me what you'd like our feedback on exactly?
> 
> So let me resume the situation:
> 
> - pre_enable_prev_first was introduced in [1]
> - some panels made use of pre_enable_prev_first
> - Visionox VTDR6130 was enabled on SM8550 systems and 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-21 Thread Dave Stevenson
Hi Dmitry

On Fri, 18 Aug 2023 at 11:27, Dmitry Baryshkov
 wrote:
>
> On 18/08/2023 11:25, neil.armstr...@linaro.org wrote:
> > Hi Dmitry,
> >
> > On 17/08/2023 20:35, Dmitry Baryshkov wrote:
> >> On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:
> >>> Hi Abhinav,
> >>>
> >>> On 14/08/2023 20:02, Abhinav Kumar wrote:
> >
> > 
> >
> >>>
> >>> Sending HS commands will always work on any controller, it's all
> >>> about LP commands.
> >>> The Samsung panels you listed only send HS commands so they can use
> >>> prepare_prev_first
> >>> and work on any controllers.
> >>
> >> I think there is some misunderstanding there, supported by the
> >> description of the flag.
> >>
> >> If I remember correctly, some hosts (sunxi) can not send DCS commands
> >> after enabling video stream and switching to HS mode, see [1]. Thus,
> >> as you know, most of the drivers have all DSI panel setup commands in
> >> drm_panel_funcs::prepare() / drm_bridge_funcs::pre_enable() callbacks,
> >> not paying attention whether these commands are to be sent in LP or in
> >> HS mode.
> >>
> >> Previously DSI source drivers could power on the DSI link either in
> >> mode_set() or in pre_enable() callbacks, with mode_set() being the
> >> hack to make panel/bridge drivers to be able to send commands from
> >> their prepare() / pre_enable() callbacks.
> >>
> >> With the prev_first flags being introduced, we have established that
> >> DSI link should be enabled in DSI host's pre_enable() callback and
> >> switched to HS mode (be it command or video) in the enable() callback.
> >>
> >> So far so good.
> >
> > It seems coherent, I would like first to have a state of all DSI host
> > drivers and make this would actually work first before adding the
> > prev_first flag to all the required panels.
> >
> >>
> >> Unfortunately this change is not fully backwards-compatible. This
> >> requires that all DSI panels sending commands from prepare() should
> >> have the prepare_prev_first flag. In some sense, all such patches
> >> might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
> >> flag to drm_panel").
> >
> > This kind of migration should be done *before* any possible regression,
> > not the other way round.
> >
> > If all panels sending commands from prepare() should have the
> > prepare_prev_first flag, then it should be first, check for regressions
> > then continue.
> >
> > 
> >
> >>>
> >>> I understand, but this patch doesn't qualify as a fix for
> >>> 9e15123eca79 and is too late to be merged in drm-misc-next for v6.6,
> >>> and since 9e15123eca79 actually breaks some support it should be
> >>> reverted (+ deps) since we are late in the rc cycles.
> >>
> >> If we go this way, we can never reapply these patches. There will be
> >> no guarantee that all panel drivers are completely converted. We
> >> already have a story without an observable end -
> >> DRM_BRIDGE_ATTACH_NO_CONNECTOR.
> >
> > I don't understand this point, who would block re-applying the patches ?
>
> Consider us reverting 9e15123eca79 now and then reapplying it next
> cycle. Then another panel / bridge that was not converted to use
> pre_enable_prev_first pops up. And suddently we have to revert them again.
>
> > The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
> > Linux version and went smoothly because we reverted
> > regressing patches and restarted when needed, I don't understand why we
> > can't do this here aswell.
>
> With DRM_BRIDGE_ATTACH_NO_CONNECTOR both host and peripheral drivers
> were involved. This way they share knowledge about the migration state.
>
> With prev_first we do not have such shared knowledge. Host assumes that
> it can work according to the documentation: turn DSI link to LP-11 in
> pre_enable(), switch to HS in enable(). It can not check whether the
> next bridge did not set pre_enable_prev_first because of it not being
> required (like for the Parade bridge) or because next bridge is not
> converted yet (and thus DSI host should power up the link in
> atomic_mode_set).
>
> Granted that there is no way for the DSI host driver to attune itself to
> the DSI peripheral driver requirements, I can only consider
> corresponding (requiring prev_first) panel drivers broken since
> 5ea6b1702781 ("drm/panel: Add prepare_prev_first flag to drm_panel") and
> all bridge drivers with this issue broken since 4fb912e5e190
> ("drm/bridge: Introduce pre_enable_prev_first to alter bridge init order").

Can I point out that even prior to 5ea6b1702781 the docs stated [1]

"Also note that those callbacks can be called no matter the state the
host is in. Drivers that need the underlying device to be powered to
perform these operations will first need to make sure it’s been
properly enabled."

added in bacbab58f09dc. So your DSI host driver isn't working in the
documented manner prior to 5ea6b1702781, therefore 5ea6b1702781
doesn't cause a regression in itself, and there was no direct
requirement for 5ea6b1702781 to add the flag to 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-21 Thread neil . armstrong

Hi Maxime,

On 21/08/2023 10:17, Maxime Ripard wrote:

Hi,

On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org wrote:

On 17/08/2023 20:35, Dmitry Baryshkov wrote:

On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:

Sending HS commands will always work on any controller, it's all
about LP commands. The Samsung panels you listed only send HS
commands so they can use prepare_prev_first and work on any
controllers.


I think there is some misunderstanding there, supported by the
description of the flag.

If I remember correctly, some hosts (sunxi) can not send DCS
commands after enabling video stream and switching to HS mode, see
[1]. Thus, as you know, most of the drivers have all DSI panel setup
commands in drm_panel_funcs::prepare() /
drm_bridge_funcs::pre_enable() callbacks, not paying attention
whether these commands are to be sent in LP or in HS mode.

Previously DSI source drivers could power on the DSI link either in
mode_set() or in pre_enable() callbacks, with mode_set() being the
hack to make panel/bridge drivers to be able to send commands from
their prepare() / pre_enable() callbacks.

With the prev_first flags being introduced, we have established that
DSI link should be enabled in DSI host's pre_enable() callback and
switched to HS mode (be it command or video) in the enable()
callback.

So far so good.


It seems coherent, I would like first to have a state of all DSI host
drivers and make this would actually work first before adding the
prev_first flag to all the required panels.


This is definitely what we should do in an ideal world, but at least for
sunxi there's no easy way for it at the moment. There's no documentation
for it and the driver provided doesn't allow this to happen.

Note that I'm not trying to discourage you or something here, I'm simply
pointing out that this will be something that we will have to take into
account. And it's possible that other drivers are in a similar
situation.


Unfortunately this change is not fully backwards-compatible. This
requires that all DSI panels sending commands from prepare() should
have the prepare_prev_first flag. In some sense, all such patches
might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
flag to drm_panel").


This kind of migration should be done *before* any possible
regression, not the other way round.

If all panels sending commands from prepare() should have the
prepare_prev_first flag, then it should be first, check for
regressions then continue.





I understand, but this patch doesn't qualify as a fix for
9e15123eca79 and is too late to be merged in drm-misc-next for
v6.6, and since 9e15123eca79 actually breaks some support it
should be reverted (+ deps) since we are late in the rc cycles.


If we go this way, we can never reapply these patches. There will be
no guarantee that all panel drivers are completely converted. We
already have a story without an observable end -
DRM_BRIDGE_ATTACH_NO_CONNECTOR.


I don't understand this point, who would block re-applying the patches ?

The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
Linux version and went smoothly because we reverted regressing patches
and restarted when needed, I don't understand why we can't do this
here aswell.


I'd consider that the DSI driver is correct here and it is about the
panel drivers that require fixes patches. If you care about the
particular Fixes tag, I have provided one several lines above.


Unfortunately it should be done in the other way round, prepare for
migration, then migrate,

I mean if it's a required migration, then it should be done and I'll
support it from both bridge and panel PoV.

So, first this patch has the wrong Fixes tag, and I would like a
better explanation on the commit message in any case. Then I would
like to have an ack from some drm-misc maintainers before applying it
because it fixes a patch that was sent via the msm tree thus per the
drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.


Sorry, it's not clear to me what you'd like our feedback on exactly?


So let me resume the situation:

- pre_enable_prev_first was introduced in [1]
- some panels made use of pre_enable_prev_first
- Visionox VTDR6130 was enabled on SM8550 systems and works on v6.5 kernels and 
before
- patch [2] was introduced on MSM DRM tree, breaking VTDR6130 on SM8550 systems 
(and probably other Video mode panels on Qcom platforms)
- this fix was sent late, and is now too late to be merged via drm-misc-next

I do not consider it's the right way to fix regression caused by [2]
I consider [2] should be reverted, panels migrated to pre_enable_prev_first 
when needed, tested and the [2] applied again

I have no objection about [2] and it should be done widely over the whole DSI 
controllers
and DSI Video panels.

I also object about the Fixes tag of this patch, which is wrong, and Dmitry 
considers [1]
should be used but it's even more wrong since [2] really caused the 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-21 Thread Maxime Ripard
Hi,

On Fri, Aug 18, 2023 at 10:25:48AM +0200, neil.armstr...@linaro.org wrote:
> On 17/08/2023 20:35, Dmitry Baryshkov wrote:
> > On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:
> > > Sending HS commands will always work on any controller, it's all
> > > about LP commands. The Samsung panels you listed only send HS
> > > commands so they can use prepare_prev_first and work on any
> > > controllers.
> > 
> > I think there is some misunderstanding there, supported by the
> > description of the flag.
> > 
> > If I remember correctly, some hosts (sunxi) can not send DCS
> > commands after enabling video stream and switching to HS mode, see
> > [1]. Thus, as you know, most of the drivers have all DSI panel setup
> > commands in drm_panel_funcs::prepare() /
> > drm_bridge_funcs::pre_enable() callbacks, not paying attention
> > whether these commands are to be sent in LP or in HS mode.
> > 
> > Previously DSI source drivers could power on the DSI link either in
> > mode_set() or in pre_enable() callbacks, with mode_set() being the
> > hack to make panel/bridge drivers to be able to send commands from
> > their prepare() / pre_enable() callbacks.
> > 
> > With the prev_first flags being introduced, we have established that
> > DSI link should be enabled in DSI host's pre_enable() callback and
> > switched to HS mode (be it command or video) in the enable()
> > callback.
> >
> > So far so good.
> 
> It seems coherent, I would like first to have a state of all DSI host
> drivers and make this would actually work first before adding the
> prev_first flag to all the required panels.

This is definitely what we should do in an ideal world, but at least for
sunxi there's no easy way for it at the moment. There's no documentation
for it and the driver provided doesn't allow this to happen.

Note that I'm not trying to discourage you or something here, I'm simply
pointing out that this will be something that we will have to take into
account. And it's possible that other drivers are in a similar
situation.

> > Unfortunately this change is not fully backwards-compatible. This
> > requires that all DSI panels sending commands from prepare() should
> > have the prepare_prev_first flag. In some sense, all such patches
> > might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first
> > flag to drm_panel").
> 
> This kind of migration should be done *before* any possible
> regression, not the other way round.
> 
> If all panels sending commands from prepare() should have the
> prepare_prev_first flag, then it should be first, check for
> regressions then continue.
> 
> 
> 
> > > 
> > > I understand, but this patch doesn't qualify as a fix for
> > > 9e15123eca79 and is too late to be merged in drm-misc-next for
> > > v6.6, and since 9e15123eca79 actually breaks some support it
> > > should be reverted (+ deps) since we are late in the rc cycles.
> > 
> > If we go this way, we can never reapply these patches. There will be
> > no guarantee that all panel drivers are completely converted. We
> > already have a story without an observable end -
> > DRM_BRIDGE_ATTACH_NO_CONNECTOR.
> 
> I don't understand this point, who would block re-applying the patches ?
> 
> The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple
> Linux version and went smoothly because we reverted regressing patches
> and restarted when needed, I don't understand why we can't do this
> here aswell.
> 
> > I'd consider that the DSI driver is correct here and it is about the
> > panel drivers that require fixes patches. If you care about the
> > particular Fixes tag, I have provided one several lines above.
> 
> Unfortunately it should be done in the other way round, prepare for
> migration, then migrate,
> 
> I mean if it's a required migration, then it should be done and I'll
> support it from both bridge and panel PoV.
> 
> So, first this patch has the wrong Fixes tag, and I would like a
> better explanation on the commit message in any case. Then I would
> like to have an ack from some drm-misc maintainers before applying it
> because it fixes a patch that was sent via the msm tree thus per the
> drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.

Sorry, it's not clear to me what you'd like our feedback on exactly?

Maxime


signature.asc
Description: PGP signature


Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-18 Thread Dmitry Baryshkov

On 18/08/2023 11:25, neil.armstr...@linaro.org wrote:

Hi Dmitry,

On 17/08/2023 20:35, Dmitry Baryshkov wrote:

On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:

Hi Abhinav,

On 14/08/2023 20:02, Abhinav Kumar wrote:






Sending HS commands will always work on any controller, it's all 
about LP commands.
The Samsung panels you listed only send HS commands so they can use 
prepare_prev_first

and work on any controllers.


I think there is some misunderstanding there, supported by the 
description of the flag.


If I remember correctly, some hosts (sunxi) can not send DCS commands 
after enabling video stream and switching to HS mode, see [1]. Thus, 
as you know, most of the drivers have all DSI panel setup commands in 
drm_panel_funcs::prepare() / drm_bridge_funcs::pre_enable() callbacks, 
not paying attention whether these commands are to be sent in LP or in 
HS mode.


Previously DSI source drivers could power on the DSI link either in 
mode_set() or in pre_enable() callbacks, with mode_set() being the 
hack to make panel/bridge drivers to be able to send commands from 
their prepare() / pre_enable() callbacks.


With the prev_first flags being introduced, we have established that 
DSI link should be enabled in DSI host's pre_enable() callback and 
switched to HS mode (be it command or video) in the enable() callback.


So far so good.


It seems coherent, I would like first to have a state of all DSI host 
drivers and make this would actually work first before adding the 
prev_first flag to all the required panels.




Unfortunately this change is not fully backwards-compatible. This 
requires that all DSI panels sending commands from prepare() should 
have the prepare_prev_first flag. In some sense, all such patches 
might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first 
flag to drm_panel").


This kind of migration should be done *before* any possible regression, 
not the other way round.


If all panels sending commands from prepare() should have the 
prepare_prev_first flag, then it should be first, check for regressions 
then continue.






I understand, but this patch doesn't qualify as a fix for 
9e15123eca79 and is too late to be merged in drm-misc-next for v6.6,
and since 9e15123eca79 actually breaks some support it should be 
reverted (+ deps) since we are late in the rc cycles.


If we go this way, we can never reapply these patches. There will be 
no guarantee that all panel drivers are completely converted. We 
already have a story without an observable end - 
DRM_BRIDGE_ATTACH_NO_CONNECTOR.


I don't understand this point, who would block re-applying the patches ?


Consider us reverting 9e15123eca79 now and then reapplying it next 
cycle. Then another panel / bridge that was not converted to use 
pre_enable_prev_first pops up. And suddently we have to revert them again.


The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple 
Linux version and went smoothly because we reverted
regressing patches and restarted when needed, I don't understand why we 
can't do this here aswell.


With DRM_BRIDGE_ATTACH_NO_CONNECTOR both host and peripheral drivers 
were involved. This way they share knowledge about the migration state.


With prev_first we do not have such shared knowledge. Host assumes that 
it can work according to the documentation: turn DSI link to LP-11 in 
pre_enable(), switch to HS in enable(). It can not check whether the 
next bridge did not set pre_enable_prev_first because of it not being 
required (like for the Parade bridge) or because next bridge is not 
converted yet (and thus DSI host should power up the link in 
atomic_mode_set).


Granted that there is no way for the DSI host driver to attune itself to 
the DSI peripheral driver requirements, I can only consider 
corresponding (requiring prev_first) panel drivers broken since 
5ea6b1702781 ("drm/panel: Add prepare_prev_first flag to drm_panel") and 
all bridge drivers with this issue broken since 4fb912e5e190 
("drm/bridge: Introduce pre_enable_prev_first to alter bridge init order").






I'd consider that the DSI driver is correct here and it is about the 
panel drivers that require fixes patches. If you care about the 
particular Fixes tag, I have provided one several lines above.


Unfortunately it should be done in the other way round, prepare for 
migration, then migrate,


I mean if it's a required migration, then it should be done and I'll 
support it from both bridge and panel PoV.


So, first this patch has the wrong Fixes tag, and I would like a better 
explanation on the commit message in any case.
Then I would like to have an ack from some drm-misc maintainers before 
applying it because it fixes a patch that
was sent via the msm tree thus per the drm-misc rules I cannot apply it 
via the drm-misc-next-fixes tree.


Neil





--
With best wishes
Dmitry



Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-18 Thread neil . armstrong

Hi Dmitry,

On 17/08/2023 20:35, Dmitry Baryshkov wrote:

On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:

Hi Abhinav,

On 14/08/2023 20:02, Abhinav Kumar wrote:






Sending HS commands will always work on any controller, it's all about LP 
commands.
The Samsung panels you listed only send HS commands so they can use 
prepare_prev_first
and work on any controllers.


I think there is some misunderstanding there, supported by the description of 
the flag.

If I remember correctly, some hosts (sunxi) can not send DCS commands after 
enabling video stream and switching to HS mode, see [1]. Thus, as you know, 
most of the drivers have all DSI panel setup commands in 
drm_panel_funcs::prepare() / drm_bridge_funcs::pre_enable() callbacks, not 
paying attention whether these commands are to be sent in LP or in HS mode.

Previously DSI source drivers could power on the DSI link either in mode_set() 
or in pre_enable() callbacks, with mode_set() being the hack to make 
panel/bridge drivers to be able to send commands from their prepare() / 
pre_enable() callbacks.

With the prev_first flags being introduced, we have established that DSI link 
should be enabled in DSI host's pre_enable() callback and switched to HS mode 
(be it command or video) in the enable() callback.

So far so good.


It seems coherent, I would like first to have a state of all DSI host drivers 
and make this would actually work first before adding the prev_first flag to 
all the required panels.



Unfortunately this change is not fully backwards-compatible. This requires that all DSI 
panels sending commands from prepare() should have the prepare_prev_first flag. In some 
sense, all such patches might have Fixes: 5ea6b1702781 ("drm/panel: Add 
prepare_prev_first flag to drm_panel").


This kind of migration should be done *before* any possible regression, not the 
other way round.

If all panels sending commands from prepare() should have the 
prepare_prev_first flag, then it should be first, check for regressions then 
continue.





I understand, but this patch doesn't qualify as a fix for 9e15123eca79 and is 
too late to be merged in drm-misc-next for v6.6,
and since 9e15123eca79 actually breaks some support it should be reverted (+ 
deps) since we are late in the rc cycles.


If we go this way, we can never reapply these patches. There will be no 
guarantee that all panel drivers are completely converted. We already have a 
story without an observable end - DRM_BRIDGE_ATTACH_NO_CONNECTOR.


I don't understand this point, who would block re-applying the patches ?

The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple Linux 
version and went smoothly because we reverted
regressing patches and restarted when needed, I don't understand why we can't 
do this here aswell.



I'd consider that the DSI driver is correct here and it is about the panel 
drivers that require fixes patches. If you care about the particular Fixes tag, 
I have provided one several lines above.


Unfortunately it should be done in the other way round, prepare for migration, 
then migrate,

I mean if it's a required migration, then it should be done and I'll support it 
from both bridge and panel PoV.

So, first this patch has the wrong Fixes tag, and I would like a better 
explanation on the commit message in any case.
Then I would like to have an ack from some drm-misc maintainers before applying 
it because it fixes a patch that
was sent via the msm tree thus per the drm-misc rules I cannot apply it via the 
drm-misc-next-fixes tree.

Neil





Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-17 Thread Dmitry Baryshkov

On 16/08/2023 10:51, neil.armstr...@linaro.org wrote:

Hi Abhinav,

On 14/08/2023 20:02, Abhinav Kumar wrote:

Hi Neil

On 8/14/2023 1:01 AM, neil.armstr...@linaro.org wrote:

Hi Abhinav,

On 10/08/2023 18:26, Abhinav Kumar wrote:

Hi Neil

On 8/3/2023 10:19 AM, Jessica Zhang wrote:



On 7/31/2023 6:00 AM, Neil Armstrong wrote:

Hi,

On 26/07/2023 00:56, Jessica Zhang wrote:
Due to a recent introduction of the pre_enable_prev_first bridge 
flag [1],
the panel driver will be probed before the DSI is enabled, 
causing the

DCS commands to fail to send.

Ensure that DSI is enabled before panel probe by setting the
prepare_prev_first flag for the panel.


Well this is specific to MSM DSI driver, it's not related at all 
to the panel.




I dont fully agree this is a MSM DSI driver specific thing.

If the panel can send its commands in its enable() callback, then 
this flag need not be set.


When a panel sends its DCS commands in its pre_enable() callback, 
any DSI controller will need to be ON before that otherwise DCS 
commands cannot be sent.


With this in mind, may I know why is this a MSM change and not a 
panel change?


As per my discussion with Dmitry during the last sync up, we were 
aligned on this expectation.


As of today, only the MSM DSI driver expects panels to have 
prepare_prev_first because it's the first
one calling pre_enable() before the DSI controller to be on, all 
other DSI drivers I know
still enables the DSI controller in mode_set() and thus can send 
commands in pre_enable() which

is a loose way to map the pre-video state for DSI panels...



It looks like there are multiple panels already setting this flag so 
this panel will not be the first unless they were added to make those 
work with MSM (which seems unlikely)


panel-samsung-s6d7aa0.c:    ctx->panel.prepare_prev_first = true;
panel-samsung-s6e3ha2.c:    ctx->panel.prepare_prev_first = true;
panel-samsung-s6e63j0x03.c: ctx->panel.prepare_prev_first = true;
panel-samsung-s6e8aa0.c:    ctx->panel.prepare_prev_first = true;

This is where I would like to understand a bit that if the panel sends 
out the ON commands in enable() instead of pre_enable() then, this 
flag will not be needed. So its also depends on the panel side and 
thats why
the bridge feeds of the panel's input in 
devm_drm_panel_bridge_add_typed()


bridge->pre_enable_prev_first = panel->prepare_prev_first;

A panel driver should not depend on features of a DSI controller, 
which is the case here
with this patch. Today's expectation is to send DSI commands in 
pre_enable() then when enabled

expect to be in video mode when enable() is called.



We are not depending on any feature as such. Any DSI controller , not 
just MSM's would need to be ON for DCS commands to be sent out in the 
panel's pre_enable() callback.


Its not true that MSM is the only driver powering on the DSI 
controller in pre_enable(). Even MTK seems to be doing that


mtk_dsi_bridge_atomic_pre_enable

So I assume any panel which sends out commands in pre_enable() will 
not work with MTK as well.


Sending HS commands will always work on any controller, it's all about 
LP commands.
The Samsung panels you listed only send HS commands so they can use 
prepare_prev_first

and work on any controllers.


I think there is some misunderstanding there, supported by the 
description of the flag.


If I remember correctly, some hosts (sunxi) can not send DCS commands 
after enabling video stream and switching to HS mode, see [1]. Thus, as 
you know, most of the drivers have all DSI panel setup commands in 
drm_panel_funcs::prepare() / drm_bridge_funcs::pre_enable() callbacks, 
not paying attention whether these commands are to be sent in LP or in 
HS mode.


Previously DSI source drivers could power on the DSI link either in 
mode_set() or in pre_enable() callbacks, with mode_set() being the hack 
to make panel/bridge drivers to be able to send commands from their 
prepare() / pre_enable() callbacks.


With the prev_first flags being introduced, we have established that DSI 
link should be enabled in DSI host's pre_enable() callback and switched 
to HS mode (be it command or video) in the enable() callback.


So far so good.

Unfortunately this change is not fully backwards-compatible. This 
requires that all DSI panels sending commands from prepare() should have 
the prepare_prev_first flag. In some sense, all such patches might have 
Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first flag to drm_panel").




None of the panels using LP commands uses prepare_prev_first:

$ grep prepare_prev_first `grep -l LPM drivers/gpu/drm/panel/panel-*`
$

Note that there's a smart move for VTDR6130 with the command mode 
introduced in 20230728011218.14630-1-parel...@quicinc.com,
in the way prepare_prev_first could only be set to true if command-mode 
is selected.
I'll accept that since it would be logical, video mode won't work 
anymore but by default

the panel would still work in command 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-16 Thread neil . armstrong

Hi Abhinav,

On 14/08/2023 20:02, Abhinav Kumar wrote:

Hi Neil

On 8/14/2023 1:01 AM, neil.armstr...@linaro.org wrote:

Hi Abhinav,

On 10/08/2023 18:26, Abhinav Kumar wrote:

Hi Neil

On 8/3/2023 10:19 AM, Jessica Zhang wrote:



On 7/31/2023 6:00 AM, Neil Armstrong wrote:

Hi,

On 26/07/2023 00:56, Jessica Zhang wrote:

Due to a recent introduction of the pre_enable_prev_first bridge flag [1],
the panel driver will be probed before the DSI is enabled, causing the
DCS commands to fail to send.

Ensure that DSI is enabled before panel probe by setting the
prepare_prev_first flag for the panel.


Well this is specific to MSM DSI driver, it's not related at all to the panel.




I dont fully agree this is a MSM DSI driver specific thing.

If the panel can send its commands in its enable() callback, then this flag 
need not be set.

When a panel sends its DCS commands in its pre_enable() callback, any DSI 
controller will need to be ON before that otherwise DCS commands cannot be sent.

With this in mind, may I know why is this a MSM change and not a panel change?

As per my discussion with Dmitry during the last sync up, we were aligned on 
this expectation.


As of today, only the MSM DSI driver expects panels to have prepare_prev_first 
because it's the first
one calling pre_enable() before the DSI controller to be on, all other DSI 
drivers I know
still enables the DSI controller in mode_set() and thus can send commands in 
pre_enable() which
is a loose way to map the pre-video state for DSI panels...



It looks like there are multiple panels already setting this flag so this panel 
will not be the first unless they were added to make those work with MSM (which 
seems unlikely)

panel-samsung-s6d7aa0.c:    ctx->panel.prepare_prev_first = true;
panel-samsung-s6e3ha2.c:    ctx->panel.prepare_prev_first = true;
panel-samsung-s6e63j0x03.c: ctx->panel.prepare_prev_first = true;
panel-samsung-s6e8aa0.c:    ctx->panel.prepare_prev_first = true;

This is where I would like to understand a bit that if the panel sends out the 
ON commands in enable() instead of pre_enable() then, this flag will not be 
needed. So its also depends on the panel side and thats why
the bridge feeds of the panel's input in devm_drm_panel_bridge_add_typed()

bridge->pre_enable_prev_first = panel->prepare_prev_first;


A panel driver should not depend on features of a DSI controller, which is the 
case here
with this patch. Today's expectation is to send DSI commands in pre_enable() 
then when enabled
expect to be in video mode when enable() is called.



We are not depending on any feature as such. Any DSI controller , not just 
MSM's would need to be ON for DCS commands to be sent out in the panel's 
pre_enable() callback.

Its not true that MSM is the only driver powering on the DSI controller in 
pre_enable(). Even MTK seems to be doing that

mtk_dsi_bridge_atomic_pre_enable

So I assume any panel which sends out commands in pre_enable() will not work 
with MTK as well.


Sending HS commands will always work on any controller, it's all about LP 
commands.
The Samsung panels you listed only send HS commands so they can use 
prepare_prev_first
and work on any controllers.

None of the panels using LP commands uses prepare_prev_first:

$ grep prepare_prev_first `grep -l LPM drivers/gpu/drm/panel/panel-*`
$

Note that there's a smart move for VTDR6130 with the command mode introduced in 
20230728011218.14630-1-parel...@quicinc.com,
in the way prepare_prev_first could only be set to true if command-mode is 
selected.
I'll accept that since it would be logical, video mode won't work anymore but 
by default
the panel would still work in command mode + prepare_prev_first.




The main reason is because some DSI controllers cannot send LP commands after 
switching
to video mode (allwinner for example), so we must take this into account.

For v6.6, I don't see other solutions than reverting 9e15123eca79 (reverting 
won't regress anything,
because now it regresses also other panels on MSM platforms) and try to find a 
proper solution for v6.7...



No, I would prefer not to revert that. It will bring back special handling for 
the parade chip into MSM driver, something which I would prefer not to go back 
to. Powering on the DSI in modeset() was done only for the parade chip.


I understand, but this patch doesn't qualify as a fix for 9e15123eca79 and is 
too late to be merged in drm-misc-next for v6.6,
and since 9e15123eca79 actually breaks some support it should be reverted (+ 
deps) since we are late in the rc cycles.

It's not a fatality or the end of the world, but this is an indirect fix and 
not way all this should be fixed.
We already had the case for the lt9611 breakage, and it's the same case here.

Neil




Neil



Thanks

Abhinav


Hi Neil,

I think there might be some confusion caused by the commit message -- instead of "enabled 
before panel probe", it should be "enabled before panel pre_enable()" as the panel 

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-14 Thread Abhinav Kumar

Hi Neil

On 8/14/2023 1:01 AM, neil.armstr...@linaro.org wrote:

Hi Abhinav,

On 10/08/2023 18:26, Abhinav Kumar wrote:

Hi Neil

On 8/3/2023 10:19 AM, Jessica Zhang wrote:



On 7/31/2023 6:00 AM, Neil Armstrong wrote:

Hi,

On 26/07/2023 00:56, Jessica Zhang wrote:
Due to a recent introduction of the pre_enable_prev_first bridge 
flag [1],

the panel driver will be probed before the DSI is enabled, causing the
DCS commands to fail to send.

Ensure that DSI is enabled before panel probe by setting the
prepare_prev_first flag for the panel.


Well this is specific to MSM DSI driver, it's not related at all to 
the panel.




I dont fully agree this is a MSM DSI driver specific thing.

If the panel can send its commands in its enable() callback, then this 
flag need not be set.


When a panel sends its DCS commands in its pre_enable() callback, any 
DSI controller will need to be ON before that otherwise DCS commands 
cannot be sent.


With this in mind, may I know why is this a MSM change and not a panel 
change?


As per my discussion with Dmitry during the last sync up, we were 
aligned on this expectation.


As of today, only the MSM DSI driver expects panels to have 
prepare_prev_first because it's the first
one calling pre_enable() before the DSI controller to be on, all other 
DSI drivers I know
still enables the DSI controller in mode_set() and thus can send 
commands in pre_enable() which

is a loose way to map the pre-video state for DSI panels...



It looks like there are multiple panels already setting this flag so 
this panel will not be the first unless they were added to make those 
work with MSM (which seems unlikely)


panel-samsung-s6d7aa0.c:ctx->panel.prepare_prev_first = true;
panel-samsung-s6e3ha2.c:ctx->panel.prepare_prev_first = true;
panel-samsung-s6e63j0x03.c: ctx->panel.prepare_prev_first = true;
panel-samsung-s6e8aa0.c:ctx->panel.prepare_prev_first = true;

This is where I would like to understand a bit that if the panel sends 
out the ON commands in enable() instead of pre_enable() then, this flag 
will not be needed. So its also depends on the panel side and thats why

the bridge feeds of the panel's input in devm_drm_panel_bridge_add_typed()

bridge->pre_enable_prev_first = panel->prepare_prev_first;

A panel driver should not depend on features of a DSI controller, which 
is the case here
with this patch. Today's expectation is to send DSI commands in 
pre_enable() then when enabled

expect to be in video mode when enable() is called.



We are not depending on any feature as such. Any DSI controller , not 
just MSM's would need to be ON for DCS commands to be sent out in the 
panel's pre_enable() callback.


Its not true that MSM is the only driver powering on the DSI controller 
in pre_enable(). Even MTK seems to be doing that


mtk_dsi_bridge_atomic_pre_enable

So I assume any panel which sends out commands in pre_enable() will not 
work with MTK as well.


The main reason is because some DSI controllers cannot send LP commands 
after switching

to video mode (allwinner for example), so we must take this into account.

For v6.6, I don't see other solutions than reverting 9e15123eca79 
(reverting won't regress anything,
because now it regresses also other panels on MSM platforms) and try to 
find a proper solution for v6.7...




No, I would prefer not to revert that. It will bring back special 
handling for the parade chip into MSM driver, something which I would 
prefer not to go back to. Powering on the DSI in modeset() was done only 
for the parade chip.



Neil



Thanks

Abhinav


Hi Neil,

I think there might be some confusion caused by the commit message -- 
instead of "enabled before panel probe", it should be "enabled before 
panel pre_enable()" as the panel on commands are sent during 
prepare(), which is matched to bridge pre_enable().


IIRC the general rule is that the panel driver should set the 
prepare_prev_first flag if the on commands are sent during 
pre_enable(), so I'll keep the code change but correct the commit 
message if that's ok with you.


Thanks,




Jessica Zhang



Neil



[1] commit 4fb912e5e190 ("drm/bridge: Introduce 
pre_enable_prev_first to alter bridge init order")


It's not the right commit that cause regression here, it's :

9e15123eca79 drm/msm/dsi: Stop unconditionally powering up DSI hosts at 
modeset




Fixes: 2349183d32d8 ("drm/panel: add visionox vtdr6130 DSI panel 
driver")

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/panel/panel-visionox-vtdr6130.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c 
b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c

index bb0dfd86ea67..e1363e128e7e 100644
--- a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
+++ b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
@@ -296,6 +296,7 @@ static int visionox_vtdr6130_probe(struct 
mipi_dsi_device *dsi)

  dsi->format = MIPI_DSI_FMT_RGB888;
  

Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-14 Thread neil . armstrong

Hi Abhinav,

On 10/08/2023 18:26, Abhinav Kumar wrote:

Hi Neil

On 8/3/2023 10:19 AM, Jessica Zhang wrote:



On 7/31/2023 6:00 AM, Neil Armstrong wrote:

Hi,

On 26/07/2023 00:56, Jessica Zhang wrote:

Due to a recent introduction of the pre_enable_prev_first bridge flag [1],
the panel driver will be probed before the DSI is enabled, causing the
DCS commands to fail to send.

Ensure that DSI is enabled before panel probe by setting the
prepare_prev_first flag for the panel.


Well this is specific to MSM DSI driver, it's not related at all to the panel.




I dont fully agree this is a MSM DSI driver specific thing.

If the panel can send its commands in its enable() callback, then this flag 
need not be set.

When a panel sends its DCS commands in its pre_enable() callback, any DSI 
controller will need to be ON before that otherwise DCS commands cannot be sent.

With this in mind, may I know why is this a MSM change and not a panel change?

As per my discussion with Dmitry during the last sync up, we were aligned on 
this expectation.


As of today, only the MSM DSI driver expects panels to have prepare_prev_first 
because it's the first
one calling pre_enable() before the DSI controller to be on, all other DSI 
drivers I know
still enables the DSI controller in mode_set() and thus can send commands in 
pre_enable() which
is a loose way to map the pre-video state for DSI panels...

A panel driver should not depend on features of a DSI controller, which is the 
case here
with this patch. Today's expectation is to send DSI commands in pre_enable() 
then when enabled
expect to be in video mode when enable() is called.

The main reason is because some DSI controllers cannot send LP commands after 
switching
to video mode (allwinner for example), so we must take this into account.

For v6.6, I don't see other solutions than reverting 9e15123eca79 (reverting 
won't regress anything,
because now it regresses also other panels on MSM platforms) and try to find a 
proper solution for v6.7...

Neil



Thanks

Abhinav


Hi Neil,

I think there might be some confusion caused by the commit message -- instead of "enabled 
before panel probe", it should be "enabled before panel pre_enable()" as the panel 
on commands are sent during prepare(), which is matched to bridge pre_enable().

IIRC the general rule is that the panel driver should set the 
prepare_prev_first flag if the on commands are sent during pre_enable(), so 
I'll keep the code change but correct the commit message if that's ok with you.

Thanks,




Jessica Zhang



Neil



[1] commit 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to alter 
bridge init order")


It's not the right commit that cause regression here, it's :

9e15123eca79 drm/msm/dsi: Stop unconditionally powering up DSI hosts at modeset



Fixes: 2349183d32d8 ("drm/panel: add visionox vtdr6130 DSI panel driver")
Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/panel/panel-visionox-vtdr6130.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c 
b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
index bb0dfd86ea67..e1363e128e7e 100644
--- a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
+++ b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
@@ -296,6 +296,7 @@ static int visionox_vtdr6130_probe(struct mipi_dsi_device 
*dsi)
  dsi->format = MIPI_DSI_FMT_RGB888;
  dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_NO_EOT_PACKET |
    MIPI_DSI_CLOCK_NON_CONTINUOUS;
+    ctx->panel.prepare_prev_first = true;
  drm_panel_init(>panel, dev, _vtdr6130_panel_funcs,
 DRM_MODE_CONNECTOR_DSI);

---
base-commit: 28a5c036b05fc5c935cc72d76abd3589825ea9cd
change-id: 20230717-visionox-vtdr-prev-first-e00ae02eec9f

Best regards,






Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-10 Thread Abhinav Kumar

Hi Neil

On 8/3/2023 10:19 AM, Jessica Zhang wrote:



On 7/31/2023 6:00 AM, Neil Armstrong wrote:

Hi,

On 26/07/2023 00:56, Jessica Zhang wrote:
Due to a recent introduction of the pre_enable_prev_first bridge flag 
[1],

the panel driver will be probed before the DSI is enabled, causing the
DCS commands to fail to send.

Ensure that DSI is enabled before panel probe by setting the
prepare_prev_first flag for the panel.


Well this is specific to MSM DSI driver, it's not related at all to 
the panel.




I dont fully agree this is a MSM DSI driver specific thing.

If the panel can send its commands in its enable() callback, then this 
flag need not be set.


When a panel sends its DCS commands in its pre_enable() callback, any 
DSI controller will need to be ON before that otherwise DCS commands 
cannot be sent.


With this in mind, may I know why is this a MSM change and not a panel 
change?


As per my discussion with Dmitry during the last sync up, we were 
aligned on this expectation.


Thanks

Abhinav


Hi Neil,

I think there might be some confusion caused by the commit message -- 
instead of "enabled before panel probe", it should be "enabled before 
panel pre_enable()" as the panel on commands are sent during prepare(), 
which is matched to bridge pre_enable().


IIRC the general rule is that the panel driver should set the 
prepare_prev_first flag if the on commands are sent during pre_enable(), 
so I'll keep the code change but correct the commit message if that's ok 
with you.


Thanks,




Jessica Zhang



Neil



[1] commit 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first 
to alter bridge init order")


Fixes: 2349183d32d8 ("drm/panel: add visionox vtdr6130 DSI panel 
driver")

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/panel/panel-visionox-vtdr6130.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c 
b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c

index bb0dfd86ea67..e1363e128e7e 100644
--- a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
+++ b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
@@ -296,6 +296,7 @@ static int visionox_vtdr6130_probe(struct 
mipi_dsi_device *dsi)

  dsi->format = MIPI_DSI_FMT_RGB888;
  dsi->mode_flags = MIPI_DSI_MODE_VIDEO | 
MIPI_DSI_MODE_NO_EOT_PACKET |

    MIPI_DSI_CLOCK_NON_CONTINUOUS;
+    ctx->panel.prepare_prev_first = true;
  drm_panel_init(>panel, dev, _vtdr6130_panel_funcs,
 DRM_MODE_CONNECTOR_DSI);

---
base-commit: 28a5c036b05fc5c935cc72d76abd3589825ea9cd
change-id: 20230717-visionox-vtdr-prev-first-e00ae02eec9f

Best regards,




Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-08-03 Thread Jessica Zhang




On 7/31/2023 6:00 AM, Neil Armstrong wrote:

Hi,

On 26/07/2023 00:56, Jessica Zhang wrote:
Due to a recent introduction of the pre_enable_prev_first bridge flag 
[1],

the panel driver will be probed before the DSI is enabled, causing the
DCS commands to fail to send.

Ensure that DSI is enabled before panel probe by setting the
prepare_prev_first flag for the panel.


Well this is specific to MSM DSI driver, it's not related at all to the 
panel.


Hi Neil,

I think there might be some confusion caused by the commit message -- 
instead of "enabled before panel probe", it should be "enabled before 
panel pre_enable()" as the panel on commands are sent during prepare(), 
which is matched to bridge pre_enable().


IIRC the general rule is that the panel driver should set the 
prepare_prev_first flag if the on commands are sent during pre_enable(), 
so I'll keep the code change but correct the commit message if that's ok 
with you.


Thanks,

Jessica Zhang



Neil



[1] commit 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first 
to alter bridge init order")


Fixes: 2349183d32d8 ("drm/panel: add visionox vtdr6130 DSI panel driver")
Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/panel/panel-visionox-vtdr6130.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c 
b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c

index bb0dfd86ea67..e1363e128e7e 100644
--- a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
+++ b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
@@ -296,6 +296,7 @@ static int visionox_vtdr6130_probe(struct 
mipi_dsi_device *dsi)

  dsi->format = MIPI_DSI_FMT_RGB888;
  dsi->mode_flags = MIPI_DSI_MODE_VIDEO | 
MIPI_DSI_MODE_NO_EOT_PACKET |

    MIPI_DSI_CLOCK_NON_CONTINUOUS;
+    ctx->panel.prepare_prev_first = true;
  drm_panel_init(>panel, dev, _vtdr6130_panel_funcs,
 DRM_MODE_CONNECTOR_DSI);

---
base-commit: 28a5c036b05fc5c935cc72d76abd3589825ea9cd
change-id: 20230717-visionox-vtdr-prev-first-e00ae02eec9f

Best regards,




Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox VTDR6130

2023-07-31 Thread Neil Armstrong

Hi,

On 26/07/2023 00:56, Jessica Zhang wrote:

Due to a recent introduction of the pre_enable_prev_first bridge flag [1],
the panel driver will be probed before the DSI is enabled, causing the
DCS commands to fail to send.

Ensure that DSI is enabled before panel probe by setting the
prepare_prev_first flag for the panel.


Well this is specific to MSM DSI driver, it's not related at all to the panel.

Neil



[1] commit 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to alter 
bridge init order")

Fixes: 2349183d32d8 ("drm/panel: add visionox vtdr6130 DSI panel driver")
Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/panel/panel-visionox-vtdr6130.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c 
b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
index bb0dfd86ea67..e1363e128e7e 100644
--- a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
+++ b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
@@ -296,6 +296,7 @@ static int visionox_vtdr6130_probe(struct mipi_dsi_device 
*dsi)
dsi->format = MIPI_DSI_FMT_RGB888;
dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_NO_EOT_PACKET |
  MIPI_DSI_CLOCK_NON_CONTINUOUS;
+   ctx->panel.prepare_prev_first = true;
  
  	drm_panel_init(>panel, dev, _vtdr6130_panel_funcs,

   DRM_MODE_CONNECTOR_DSI);

---
base-commit: 28a5c036b05fc5c935cc72d76abd3589825ea9cd
change-id: 20230717-visionox-vtdr-prev-first-e00ae02eec9f

Best regards,