linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/msm/dsi: Set panel orientation when directly connected
@ 2022-07-06 19:14 Stephen Boyd
  2022-07-07 21:11 ` Abhinav Kumar
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Stephen Boyd @ 2022-07-06 19:14 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov
  Cc: linux-kernel, patches, Sean Paul, dri-devel, freedreno,
	Hsin-Yi Wang, Douglas Anderson

Set the panel orientation in drm when the panel is directly connected,
i.e. we're not using an external bridge. The external bridge case is
already handled by the panel bridge code, so we only update the path we
take when the panel is directly connected/internal. This silences a
warning splat coming from __drm_mode_object_add() on Wormdingler boards.

Cc: Hsin-Yi Wang <hsinyi@chromium.org>
Cc: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---

This relies on commit 5e41b01a7808 ("drm/panel: Add an API to allow drm
to set orientation from panel") which is in drm-misc

 drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index cb84d185d73a..9333f7095acd 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -268,6 +268,8 @@ static int msm_dsi_manager_panel_init(struct drm_connector *conn, u8 id)
 		return PTR_ERR(panel);
 	}
 
+	drm_connector_set_orientation_from_panel(conn, panel);
+
 	if (!panel || !IS_BONDED_DSI())
 		goto out;
 

base-commit: 15b9ca1641f0c3cd74885280331e9172c62a125e
-- 
https://chromeos.dev


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/msm/dsi: Set panel orientation when directly connected
  2022-07-06 19:14 [PATCH] drm/msm/dsi: Set panel orientation when directly connected Stephen Boyd
@ 2022-07-07 21:11 ` Abhinav Kumar
  2022-07-07 21:21   ` Stephen Boyd
  2022-07-08 15:25 ` Doug Anderson
  2022-09-01  8:42 ` Dmitry Baryshkov
  2 siblings, 1 reply; 14+ messages in thread
From: Abhinav Kumar @ 2022-07-07 21:11 UTC (permalink / raw)
  To: Stephen Boyd, Rob Clark, Dmitry Baryshkov
  Cc: linux-kernel, patches, Sean Paul, dri-devel, freedreno,
	Hsin-Yi Wang, Douglas Anderson



On 7/6/2022 12:14 PM, Stephen Boyd wrote:
> Set the panel orientation in drm when the panel is directly connected,
> i.e. we're not using an external bridge. The external bridge case is
> already handled by the panel bridge code, so we only update the path we
> take when the panel is directly connected/internal. This silences a
> warning splat coming from __drm_mode_object_add() on Wormdingler boards.
> 
> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
> 
> This relies on commit 5e41b01a7808 ("drm/panel: Add an API to allow drm
> to set orientation from panel") which is in drm-misc
> 
>   drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index cb84d185d73a..9333f7095acd 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -268,6 +268,8 @@ static int msm_dsi_manager_panel_init(struct drm_connector *conn, u8 id)
>   		return PTR_ERR(panel);
>   	}
>   
> +	drm_connector_set_orientation_from_panel(conn, panel);
> +

This should be moved below the !panel check since you are passing panel 
as one of the params.

I looked up the doc and it says that for unknown(default cases) this is 
a no-op so I think this change is fine otherwise.

"It is allowed to call this function with a panel_orientation of 
DRM_MODE_PANEL_ORIENTATION_UNKNOWN, in which case it is a no-op."


>   	if (!panel || !IS_BONDED_DSI())
>   		goto out;
>   
> 
> base-commit: 15b9ca1641f0c3cd74885280331e9172c62a125e

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/msm/dsi: Set panel orientation when directly connected
  2022-07-07 21:11 ` Abhinav Kumar
@ 2022-07-07 21:21   ` Stephen Boyd
  2022-07-07 21:25     ` [Freedreno] " Abhinav Kumar
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Boyd @ 2022-07-07 21:21 UTC (permalink / raw)
  To: Abhinav Kumar, Dmitry Baryshkov, Rob Clark
  Cc: linux-kernel, patches, Sean Paul, dri-devel, freedreno,
	Hsin-Yi Wang, Douglas Anderson

Quoting Abhinav Kumar (2022-07-07 14:11:08)
>
>
> On 7/6/2022 12:14 PM, Stephen Boyd wrote:
> > Set the panel orientation in drm when the panel is directly connected,
> > i.e. we're not using an external bridge. The external bridge case is
> > already handled by the panel bridge code, so we only update the path we
> > take when the panel is directly connected/internal. This silences a
> > warning splat coming from __drm_mode_object_add() on Wormdingler boards.
> >
> > Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> > Cc: Douglas Anderson <dianders@chromium.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> >
> > This relies on commit 5e41b01a7808 ("drm/panel: Add an API to allow drm
> > to set orientation from panel") which is in drm-misc
> >
> >   drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > index cb84d185d73a..9333f7095acd 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > @@ -268,6 +268,8 @@ static int msm_dsi_manager_panel_init(struct drm_connector *conn, u8 id)
> >               return PTR_ERR(panel);
> >       }
> >
> > +     drm_connector_set_orientation_from_panel(conn, panel);
> > +
>
> This should be moved below the !panel check since you are passing panel
> as one of the params.

drm_connector_set_orientation_from_panel() checks for a NULL panel
pointer and sets to UNKNOWN. If I moved this below the !panel check then
I'd have to split that condition for !IS_BONDED_DSI() which was more
diff.

>
> I looked up the doc and it says that for unknown(default cases) this is
> a no-op so I think this change is fine otherwise.
>
> "It is allowed to call this function with a panel_orientation of
> DRM_MODE_PANEL_ORIENTATION_UNKNOWN, in which case it is a no-op."

Ok, so you're fine with this patch?

>
>
> >       if (!panel || !IS_BONDED_DSI())
> >               goto out;
> >
> >
> > base-commit: 15b9ca1641f0c3cd74885280331e9172c62a125e

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Freedreno] [PATCH] drm/msm/dsi: Set panel orientation when directly connected
  2022-07-07 21:21   ` Stephen Boyd
@ 2022-07-07 21:25     ` Abhinav Kumar
  0 siblings, 0 replies; 14+ messages in thread
From: Abhinav Kumar @ 2022-07-07 21:25 UTC (permalink / raw)
  To: Stephen Boyd, Dmitry Baryshkov, Rob Clark
  Cc: Douglas Anderson, Sean Paul, linux-kernel, dri-devel, patches,
	Hsin-Yi Wang, freedreno



On 7/7/2022 2:21 PM, Stephen Boyd wrote:
> Quoting Abhinav Kumar (2022-07-07 14:11:08)
>>
>>
>> On 7/6/2022 12:14 PM, Stephen Boyd wrote:
>>> Set the panel orientation in drm when the panel is directly connected,
>>> i.e. we're not using an external bridge. The external bridge case is
>>> already handled by the panel bridge code, so we only update the path we
>>> take when the panel is directly connected/internal. This silences a
>>> warning splat coming from __drm_mode_object_add() on Wormdingler boards.
>>>
>>> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
>>> Cc: Douglas Anderson <dianders@chromium.org>
>>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>>> ---
>>>
>>> This relies on commit 5e41b01a7808 ("drm/panel: Add an API to allow drm
>>> to set orientation from panel") which is in drm-misc
>>>
>>>    drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
>>> index cb84d185d73a..9333f7095acd 100644
>>> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
>>> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
>>> @@ -268,6 +268,8 @@ static int msm_dsi_manager_panel_init(struct drm_connector *conn, u8 id)
>>>                return PTR_ERR(panel);
>>>        }
>>>
>>> +     drm_connector_set_orientation_from_panel(conn, panel);
>>> +
>>
>> This should be moved below the !panel check since you are passing panel
>> as one of the params.
> 
> drm_connector_set_orientation_from_panel() checks for a NULL panel
> pointer and sets to UNKNOWN. If I moved this below the !panel check then
> I'd have to split that condition for !IS_BONDED_DSI() which was more
> diff.
> 

Ah okay. Even if the panel is null, it sets it to 
DRM_MODE_PANEL_ORIENTATION_UNKNOWN.

>>
>> I looked up the doc and it says that for unknown(default cases) this is
>> a no-op so I think this change is fine otherwise.
>>
>> "It is allowed to call this function with a panel_orientation of
>> DRM_MODE_PANEL_ORIENTATION_UNKNOWN, in which case it is a no-op."
> 
> Ok, so you're fine with this patch?

Yes, I only held back the R-b for the previous comment, now since thats 
clarified.

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> 
>>
>>
>>>        if (!panel || !IS_BONDED_DSI())
>>>                goto out;
>>>
>>>
>>> base-commit: 15b9ca1641f0c3cd74885280331e9172c62a125e

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/msm/dsi: Set panel orientation when directly connected
  2022-07-06 19:14 [PATCH] drm/msm/dsi: Set panel orientation when directly connected Stephen Boyd
  2022-07-07 21:11 ` Abhinav Kumar
@ 2022-07-08 15:25 ` Doug Anderson
  2022-07-08 16:00   ` Abhinav Kumar
  2022-07-20 20:46   ` Rob Clark
  2022-09-01  8:42 ` Dmitry Baryshkov
  2 siblings, 2 replies; 14+ messages in thread
From: Doug Anderson @ 2022-07-08 15:25 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, LKML, patches,
	Sean Paul, dri-devel, freedreno, Hsin-Yi Wang

Hi,

On Wed, Jul 6, 2022 at 12:14 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Set the panel orientation in drm when the panel is directly connected,
> i.e. we're not using an external bridge. The external bridge case is
> already handled by the panel bridge code, so we only update the path we
> take when the panel is directly connected/internal. This silences a
> warning splat coming from __drm_mode_object_add() on Wormdingler boards.
>
> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>
> This relies on commit 5e41b01a7808 ("drm/panel: Add an API to allow drm
> to set orientation from panel") which is in drm-misc
>
>  drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 ++
>  1 file changed, 2 insertions(+)

I don't personally have objections to this, but (to my understanding)
"the future" is that everyone should use panel_bridge. If we made the
move to panel_bridge today then we wouldn't need to do this. In
general I think panel_bridge would end up letting us delete a bunch of
code...

See commit 4e5763f03e10 ("drm/bridge: ti-sn65dsi86: Wrap panel with
panel-bridge") for when this was done by ti-sn65dsi86.

Then again, I spent a small amount of time looking into this and it's
definitely non-trivial. Still likely worthwhile, but not worth
blocking a tiny fix like this. It also should be fairly obvious that
we should delete this when we switch to panel_bridge.

Thus:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

I'll assume that we'll just snooze this commit until drm-misc-next
merges into a tree that msm-next is based on, which will probably be
the next -rc1. If desired and Acked I could land this in
drm-misc-next, but it's probably not worth it?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/msm/dsi: Set panel orientation when directly connected
  2022-07-08 15:25 ` Doug Anderson
@ 2022-07-08 16:00   ` Abhinav Kumar
  2022-07-08 19:42     ` Abhinav Kumar
  2022-07-20 20:46   ` Rob Clark
  1 sibling, 1 reply; 14+ messages in thread
From: Abhinav Kumar @ 2022-07-08 16:00 UTC (permalink / raw)
  To: Doug Anderson, Stephen Boyd
  Cc: Rob Clark, Dmitry Baryshkov, LKML, patches, Sean Paul, dri-devel,
	freedreno, Hsin-Yi Wang



On 7/8/2022 8:25 AM, Doug Anderson wrote:
> Hi,
> 
> On Wed, Jul 6, 2022 at 12:14 PM Stephen Boyd <swboyd@chromium.org> wrote:
>>
>> Set the panel orientation in drm when the panel is directly connected,
>> i.e. we're not using an external bridge. The external bridge case is
>> already handled by the panel bridge code, so we only update the path we
>> take when the panel is directly connected/internal. This silences a
>> warning splat coming from __drm_mode_object_add() on Wormdingler boards.
>>
>> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
>> Cc: Douglas Anderson <dianders@chromium.org>
>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>> ---
>>
>> This relies on commit 5e41b01a7808 ("drm/panel: Add an API to allow drm
>> to set orientation from panel") which is in drm-misc
>>
>>   drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 ++
>>   1 file changed, 2 insertions(+)
> 
> I don't personally have objections to this, but (to my understanding)
> "the future" is that everyone should use panel_bridge. If we made the
> move to panel_bridge today then we wouldn't need to do this. In
> general I think panel_bridge would end up letting us delete a bunch of
> code...
> 
> See commit 4e5763f03e10 ("drm/bridge: ti-sn65dsi86: Wrap panel with
> panel-bridge") for when this was done by ti-sn65dsi86.
> 
> Then again, I spent a small amount of time looking into this and it's
> definitely non-trivial. Still likely worthwhile, but not worth
> blocking a tiny fix like this. It also should be fairly obvious that
> we should delete this when we switch to panel_bridge.

Right, from what I saw on IRC, panel_bridge is the way forward and 
dmitry did push a change to do that

https://patchwork.freedesktop.org/patch/492585/

But I think we can go ahead with this change because its simple enough.

Regarding the panel_bridge migration, I am going to start reviewing that 
as well.

> 
> Thus:
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> 
> I'll assume that we'll just snooze this commit until drm-misc-next
> merges into a tree that msm-next is based on, which will probably be
> the next -rc1. If desired and Acked I could land this in
> drm-misc-next, but it's probably not worth it?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/msm/dsi: Set panel orientation when directly connected
  2022-07-08 16:00   ` Abhinav Kumar
@ 2022-07-08 19:42     ` Abhinav Kumar
  2022-07-08 20:58       ` Dmitry Baryshkov
  0 siblings, 1 reply; 14+ messages in thread
From: Abhinav Kumar @ 2022-07-08 19:42 UTC (permalink / raw)
  To: Doug Anderson, Stephen Boyd
  Cc: Rob Clark, Dmitry Baryshkov, LKML, patches, Sean Paul, dri-devel,
	freedreno, Hsin-Yi Wang



On 7/8/2022 9:00 AM, Abhinav Kumar wrote:
> 
> 
> On 7/8/2022 8:25 AM, Doug Anderson wrote:
>> Hi,
>>
>> On Wed, Jul 6, 2022 at 12:14 PM Stephen Boyd <swboyd@chromium.org> wrote:
>>>
>>> Set the panel orientation in drm when the panel is directly connected,
>>> i.e. we're not using an external bridge. The external bridge case is
>>> already handled by the panel bridge code, so we only update the path we
>>> take when the panel is directly connected/internal. This silences a
>>> warning splat coming from __drm_mode_object_add() on Wormdingler boards.
>>>
>>> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
>>> Cc: Douglas Anderson <dianders@chromium.org>
>>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>>> ---
>>>
>>> This relies on commit 5e41b01a7808 ("drm/panel: Add an API to allow drm
>>> to set orientation from panel") which is in drm-misc
>>>
>>>   drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>
>> I don't personally have objections to this, but (to my understanding)
>> "the future" is that everyone should use panel_bridge. If we made the
>> move to panel_bridge today then we wouldn't need to do this. In
>> general I think panel_bridge would end up letting us delete a bunch of
>> code...
>>
>> See commit 4e5763f03e10 ("drm/bridge: ti-sn65dsi86: Wrap panel with
>> panel-bridge") for when this was done by ti-sn65dsi86.
>>
>> Then again, I spent a small amount of time looking into this and it's
>> definitely non-trivial. Still likely worthwhile, but not worth
>> blocking a tiny fix like this. It also should be fairly obvious that
>> we should delete this when we switch to panel_bridge.
> 
> Right, from what I saw on IRC, panel_bridge is the way forward and 
> dmitry did push a change to do that
> 
> https://patchwork.freedesktop.org/patch/492585/
> 
> But I think we can go ahead with this change because its simple enough.
> 
> Regarding the panel_bridge migration, I am going to start reviewing that 
> as well.
> 

I did some more digging up on the panel_bridge migration.

Dmitry has posted this towards december last year

https://patches.linaro.org/project/dri-devel/patch/20211207222901.988484-3-dmitry.baryshkov@linaro.org/ 


and I had given my R-b on this already in Jan.

I am not sure why this change was dropped OR was not part of msm-next 
already.

Dmitry, any reason this change was left out so long and why the R-b was 
not retained and this was reposted?

 From what i can see the change looks identical.

Thanks

Abhinav
>>
>> Thus:
>>
>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>
>> I'll assume that we'll just snooze this commit until drm-misc-next
>> merges into a tree that msm-next is based on, which will probably be
>> the next -rc1. If desired and Acked I could land this in
>> drm-misc-next, but it's probably not worth it?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/msm/dsi: Set panel orientation when directly connected
  2022-07-08 19:42     ` Abhinav Kumar
@ 2022-07-08 20:58       ` Dmitry Baryshkov
  2022-07-08 21:17         ` Abhinav Kumar
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Baryshkov @ 2022-07-08 20:58 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: Doug Anderson, Stephen Boyd, Rob Clark, LKML, patches, Sean Paul,
	dri-devel, freedreno, Hsin-Yi Wang

On Fri, 8 Jul 2022 at 22:42, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>
>
>
> On 7/8/2022 9:00 AM, Abhinav Kumar wrote:
> >
> >
> > On 7/8/2022 8:25 AM, Doug Anderson wrote:
> >> Hi,
> >>
> >> On Wed, Jul 6, 2022 at 12:14 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >>>
> >>> Set the panel orientation in drm when the panel is directly connected,
> >>> i.e. we're not using an external bridge. The external bridge case is
> >>> already handled by the panel bridge code, so we only update the path we
> >>> take when the panel is directly connected/internal. This silences a
> >>> warning splat coming from __drm_mode_object_add() on Wormdingler boards.
> >>>
> >>> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> >>> Cc: Douglas Anderson <dianders@chromium.org>
> >>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> >>> ---
> >>>
> >>> This relies on commit 5e41b01a7808 ("drm/panel: Add an API to allow drm
> >>> to set orientation from panel") which is in drm-misc
> >>>
> >>>   drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 ++
> >>>   1 file changed, 2 insertions(+)
> >>
> >> I don't personally have objections to this, but (to my understanding)
> >> "the future" is that everyone should use panel_bridge. If we made the
> >> move to panel_bridge today then we wouldn't need to do this. In
> >> general I think panel_bridge would end up letting us delete a bunch of
> >> code...
> >>
> >> See commit 4e5763f03e10 ("drm/bridge: ti-sn65dsi86: Wrap panel with
> >> panel-bridge") for when this was done by ti-sn65dsi86.
> >>
> >> Then again, I spent a small amount of time looking into this and it's
> >> definitely non-trivial. Still likely worthwhile, but not worth
> >> blocking a tiny fix like this. It also should be fairly obvious that
> >> we should delete this when we switch to panel_bridge.
> >
> > Right, from what I saw on IRC, panel_bridge is the way forward and
> > dmitry did push a change to do that
> >
> > https://patchwork.freedesktop.org/patch/492585/
> >
> > But I think we can go ahead with this change because its simple enough.
> >
> > Regarding the panel_bridge migration, I am going to start reviewing that
> > as well.
> >
>
> I did some more digging up on the panel_bridge migration.
>
> Dmitry has posted this towards december last year
>
> https://patches.linaro.org/project/dri-devel/patch/20211207222901.988484-3-dmitry.baryshkov@linaro.org/
>
>
> and I had given my R-b on this already in Jan.
>
> I am not sure why this change was dropped OR was not part of msm-next
> already.
>
> Dmitry, any reason this change was left out so long and why the R-b was
> not retained and this was reposted?
>
>  From what i can see the change looks identical.

I don't remember if it is identical or not. Basically it was postponed
to allow DSC to flow in. We used drm_panel to pass DSC pps data. And
if we use panel-bridge, we don't get a handle of the panel.
Later on I have posted the series moving DSC pps pointer from
drm_panel to mipi_dsi_device (which is logical anyway, since it's not
only the panel, who can provide the DSC pps info, some bridges can
process DSC-compressed data). But since that time it received no
feedback.


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/msm/dsi: Set panel orientation when directly connected
  2022-07-08 20:58       ` Dmitry Baryshkov
@ 2022-07-08 21:17         ` Abhinav Kumar
  0 siblings, 0 replies; 14+ messages in thread
From: Abhinav Kumar @ 2022-07-08 21:17 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Doug Anderson, Stephen Boyd, Rob Clark, LKML, patches, Sean Paul,
	dri-devel, freedreno, Hsin-Yi Wang



On 7/8/2022 1:58 PM, Dmitry Baryshkov wrote:
> On Fri, 8 Jul 2022 at 22:42, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>>
>>
>>
>> On 7/8/2022 9:00 AM, Abhinav Kumar wrote:
>>>
>>>
>>> On 7/8/2022 8:25 AM, Doug Anderson wrote:
>>>> Hi,
>>>>
>>>> On Wed, Jul 6, 2022 at 12:14 PM Stephen Boyd <swboyd@chromium.org> wrote:
>>>>>
>>>>> Set the panel orientation in drm when the panel is directly connected,
>>>>> i.e. we're not using an external bridge. The external bridge case is
>>>>> already handled by the panel bridge code, so we only update the path we
>>>>> take when the panel is directly connected/internal. This silences a
>>>>> warning splat coming from __drm_mode_object_add() on Wormdingler boards.
>>>>>
>>>>> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
>>>>> Cc: Douglas Anderson <dianders@chromium.org>
>>>>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>>>>> ---
>>>>>
>>>>> This relies on commit 5e41b01a7808 ("drm/panel: Add an API to allow drm
>>>>> to set orientation from panel") which is in drm-misc
>>>>>
>>>>>    drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 ++
>>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> I don't personally have objections to this, but (to my understanding)
>>>> "the future" is that everyone should use panel_bridge. If we made the
>>>> move to panel_bridge today then we wouldn't need to do this. In
>>>> general I think panel_bridge would end up letting us delete a bunch of
>>>> code...
>>>>
>>>> See commit 4e5763f03e10 ("drm/bridge: ti-sn65dsi86: Wrap panel with
>>>> panel-bridge") for when this was done by ti-sn65dsi86.
>>>>
>>>> Then again, I spent a small amount of time looking into this and it's
>>>> definitely non-trivial. Still likely worthwhile, but not worth
>>>> blocking a tiny fix like this. It also should be fairly obvious that
>>>> we should delete this when we switch to panel_bridge.
>>>
>>> Right, from what I saw on IRC, panel_bridge is the way forward and
>>> dmitry did push a change to do that
>>>
>>> https://patchwork.freedesktop.org/patch/492585/
>>>
>>> But I think we can go ahead with this change because its simple enough.
>>>
>>> Regarding the panel_bridge migration, I am going to start reviewing that
>>> as well.
>>>
>>
>> I did some more digging up on the panel_bridge migration.
>>
>> Dmitry has posted this towards december last year
>>
>> https://patches.linaro.org/project/dri-devel/patch/20211207222901.988484-3-dmitry.baryshkov@linaro.org/
>>
>>
>> and I had given my R-b on this already in Jan.
>>
>> I am not sure why this change was dropped OR was not part of msm-next
>> already.
>>
>> Dmitry, any reason this change was left out so long and why the R-b was
>> not retained and this was reposted?
>>
>>   From what i can see the change looks identical.
> 
> I don't remember if it is identical or not. Basically it was postponed
> to allow DSC to flow in. We used drm_panel to pass DSC pps data. And
> if we use panel-bridge, we don't get a handle of the panel.
> Later on I have posted the series moving DSC pps pointer from
> drm_panel to mipi_dsi_device (which is logical anyway, since it's not
> only the panel, who can provide the DSC pps info, some bridges can
> process DSC-compressed data). But since that time it received no
> feedback.

Thanks for the details.

I will try to review the DRM core bits. But I guess for them to land, we 
need some of the core maintainers to pitch in on that.


> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/msm/dsi: Set panel orientation when directly connected
  2022-07-08 15:25 ` Doug Anderson
  2022-07-08 16:00   ` Abhinav Kumar
@ 2022-07-20 20:46   ` Rob Clark
  2022-07-20 22:42     ` Doug Anderson
  1 sibling, 1 reply; 14+ messages in thread
From: Rob Clark @ 2022-07-20 20:46 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Stephen Boyd, Abhinav Kumar, Dmitry Baryshkov, LKML, patches,
	Sean Paul, dri-devel, freedreno, Hsin-Yi Wang

On Fri, Jul 8, 2022 at 8:25 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Wed, Jul 6, 2022 at 12:14 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > Set the panel orientation in drm when the panel is directly connected,
> > i.e. we're not using an external bridge. The external bridge case is
> > already handled by the panel bridge code, so we only update the path we
> > take when the panel is directly connected/internal. This silences a
> > warning splat coming from __drm_mode_object_add() on Wormdingler boards.
> >
> > Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> > Cc: Douglas Anderson <dianders@chromium.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> >
> > This relies on commit 5e41b01a7808 ("drm/panel: Add an API to allow drm
> > to set orientation from panel") which is in drm-misc
> >
> >  drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 ++
> >  1 file changed, 2 insertions(+)
>
> I don't personally have objections to this, but (to my understanding)
> "the future" is that everyone should use panel_bridge. If we made the
> move to panel_bridge today then we wouldn't need to do this. In
> general I think panel_bridge would end up letting us delete a bunch of
> code...
>
> See commit 4e5763f03e10 ("drm/bridge: ti-sn65dsi86: Wrap panel with
> panel-bridge") for when this was done by ti-sn65dsi86.
>
> Then again, I spent a small amount of time looking into this and it's
> definitely non-trivial. Still likely worthwhile, but not worth
> blocking a tiny fix like this. It also should be fairly obvious that
> we should delete this when we switch to panel_bridge.
>
> Thus:
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
> I'll assume that we'll just snooze this commit until drm-misc-next
> merges into a tree that msm-next is based on, which will probably be
> the next -rc1. If desired and Acked I could land this in
> drm-misc-next, but it's probably not worth it?

if you want to land this patch via drm-misc, which might be the
easier/faster route, then:

Acked-by: Rob Clark <robdclark@gmail.com>

BR,
-R

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/msm/dsi: Set panel orientation when directly connected
  2022-07-20 20:46   ` Rob Clark
@ 2022-07-20 22:42     ` Doug Anderson
  2022-08-17 20:48       ` Doug Anderson
  0 siblings, 1 reply; 14+ messages in thread
From: Doug Anderson @ 2022-07-20 22:42 UTC (permalink / raw)
  To: Rob Clark
  Cc: Stephen Boyd, Abhinav Kumar, Dmitry Baryshkov, LKML, patches,
	Sean Paul, dri-devel, freedreno, Hsin-Yi Wang

Hi,

On Wed, Jul 20, 2022 at 1:46 PM Rob Clark <robdclark@gmail.com> wrote:
>
> On Fri, Jul 8, 2022 at 8:25 AM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Wed, Jul 6, 2022 at 12:14 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > >
> > > Set the panel orientation in drm when the panel is directly connected,
> > > i.e. we're not using an external bridge. The external bridge case is
> > > already handled by the panel bridge code, so we only update the path we
> > > take when the panel is directly connected/internal. This silences a
> > > warning splat coming from __drm_mode_object_add() on Wormdingler boards.
> > >
> > > Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> > > Cc: Douglas Anderson <dianders@chromium.org>
> > > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > > ---
> > >
> > > This relies on commit 5e41b01a7808 ("drm/panel: Add an API to allow drm
> > > to set orientation from panel") which is in drm-misc
> > >
> > >  drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> >
> > I don't personally have objections to this, but (to my understanding)
> > "the future" is that everyone should use panel_bridge. If we made the
> > move to panel_bridge today then we wouldn't need to do this. In
> > general I think panel_bridge would end up letting us delete a bunch of
> > code...
> >
> > See commit 4e5763f03e10 ("drm/bridge: ti-sn65dsi86: Wrap panel with
> > panel-bridge") for when this was done by ti-sn65dsi86.
> >
> > Then again, I spent a small amount of time looking into this and it's
> > definitely non-trivial. Still likely worthwhile, but not worth
> > blocking a tiny fix like this. It also should be fairly obvious that
> > we should delete this when we switch to panel_bridge.
> >
> > Thus:
> >
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> >
> > I'll assume that we'll just snooze this commit until drm-misc-next
> > merges into a tree that msm-next is based on, which will probably be
> > the next -rc1. If desired and Acked I could land this in
> > drm-misc-next, but it's probably not worth it?
>
> if you want to land this patch via drm-misc, which might be the
> easier/faster route, then:
>
> Acked-by: Rob Clark <robdclark@gmail.com>

As per discussion on IRC, I'm not going to apply this to drm-misc-next.

Given where we are in the cycle landing in drm-misc-next means it
won't be in mainline for a couple versions and I suspect that'll cause
merge conflicts with Dmitry's series [1]. ...and, of course, if
Dmitry's series lands then we don't even need ${SUBJECT} patch...

So I think the plan is:

1. Snooze waiting for the next -rc1 since
drm_connector_set_orientation_from_panel() won't be in mainline until
then.

2. If Dmitry's series looks like a long way off, we could land
${SUBJECT} patch in msm-next as a stopgap fix.


[1] https://lore.kernel.org/r/20220711094320.368062-5-dmitry.baryshkov@linaro.org/

-Doug

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/msm/dsi: Set panel orientation when directly connected
  2022-07-20 22:42     ` Doug Anderson
@ 2022-08-17 20:48       ` Doug Anderson
  2022-08-18 23:11         ` [Freedreno] " Abhinav Kumar
  0 siblings, 1 reply; 14+ messages in thread
From: Doug Anderson @ 2022-08-17 20:48 UTC (permalink / raw)
  To: Rob Clark
  Cc: Stephen Boyd, Abhinav Kumar, Dmitry Baryshkov, LKML, patches,
	Sean Paul, dri-devel, freedreno, Hsin-Yi Wang

Hi,

On Wed, Jul 20, 2022 at 3:42 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Wed, Jul 20, 2022 at 1:46 PM Rob Clark <robdclark@gmail.com> wrote:
> >
> > On Fri, Jul 8, 2022 at 8:25 AM Doug Anderson <dianders@chromium.org> wrote:
> > >
> > > Hi,
> > >
> > > On Wed, Jul 6, 2022 at 12:14 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > > >
> > > > Set the panel orientation in drm when the panel is directly connected,
> > > > i.e. we're not using an external bridge. The external bridge case is
> > > > already handled by the panel bridge code, so we only update the path we
> > > > take when the panel is directly connected/internal. This silences a
> > > > warning splat coming from __drm_mode_object_add() on Wormdingler boards.
> > > >
> > > > Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> > > > Cc: Douglas Anderson <dianders@chromium.org>
> > > > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > > > ---
> > > >
> > > > This relies on commit 5e41b01a7808 ("drm/panel: Add an API to allow drm
> > > > to set orientation from panel") which is in drm-misc
> > > >
> > > >  drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > >
> > > I don't personally have objections to this, but (to my understanding)
> > > "the future" is that everyone should use panel_bridge. If we made the
> > > move to panel_bridge today then we wouldn't need to do this. In
> > > general I think panel_bridge would end up letting us delete a bunch of
> > > code...
> > >
> > > See commit 4e5763f03e10 ("drm/bridge: ti-sn65dsi86: Wrap panel with
> > > panel-bridge") for when this was done by ti-sn65dsi86.
> > >
> > > Then again, I spent a small amount of time looking into this and it's
> > > definitely non-trivial. Still likely worthwhile, but not worth
> > > blocking a tiny fix like this. It also should be fairly obvious that
> > > we should delete this when we switch to panel_bridge.
> > >
> > > Thus:
> > >
> > > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > >
> > > I'll assume that we'll just snooze this commit until drm-misc-next
> > > merges into a tree that msm-next is based on, which will probably be
> > > the next -rc1. If desired and Acked I could land this in
> > > drm-misc-next, but it's probably not worth it?
> >
> > if you want to land this patch via drm-misc, which might be the
> > easier/faster route, then:
> >
> > Acked-by: Rob Clark <robdclark@gmail.com>
>
> As per discussion on IRC, I'm not going to apply this to drm-misc-next.
>
> Given where we are in the cycle landing in drm-misc-next means it
> won't be in mainline for a couple versions and I suspect that'll cause
> merge conflicts with Dmitry's series [1]. ...and, of course, if
> Dmitry's series lands then we don't even need ${SUBJECT} patch...
>
> So I think the plan is:
>
> 1. Snooze waiting for the next -rc1 since
> drm_connector_set_orientation_from_panel() won't be in mainline until
> then.
>
> 2. If Dmitry's series looks like a long way off, we could land
> ${SUBJECT} patch in msm-next as a stopgap fix.
>
>
> [1] https://lore.kernel.org/r/20220711094320.368062-5-dmitry.baryshkov@linaro.org/

Just checking up. What's the latest thinking here? Do we want to land
Stephen's change as a stopgap?
drm_connector_set_orientation_from_panel() is available in v6.0-rc1.

-Doug

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Freedreno] [PATCH] drm/msm/dsi: Set panel orientation when directly connected
  2022-08-17 20:48       ` Doug Anderson
@ 2022-08-18 23:11         ` Abhinav Kumar
  0 siblings, 0 replies; 14+ messages in thread
From: Abhinav Kumar @ 2022-08-18 23:11 UTC (permalink / raw)
  To: Doug Anderson, Rob Clark
  Cc: freedreno, LKML, dri-devel, patches, Hsin-Yi Wang,
	Dmitry Baryshkov, Stephen Boyd, Sean Paul

Hi Doug

On 8/17/2022 1:48 PM, Doug Anderson wrote:
> Hi,
> 
> On Wed, Jul 20, 2022 at 3:42 PM Doug Anderson <dianders@chromium.org> wrote:
>>
>> Hi,
>>
>> On Wed, Jul 20, 2022 at 1:46 PM Rob Clark <robdclark@gmail.com> wrote:
>>>
>>> On Fri, Jul 8, 2022 at 8:25 AM Doug Anderson <dianders@chromium.org> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Wed, Jul 6, 2022 at 12:14 PM Stephen Boyd <swboyd@chromium.org> wrote:
>>>>>
>>>>> Set the panel orientation in drm when the panel is directly connected,
>>>>> i.e. we're not using an external bridge. The external bridge case is
>>>>> already handled by the panel bridge code, so we only update the path we
>>>>> take when the panel is directly connected/internal. This silences a
>>>>> warning splat coming from __drm_mode_object_add() on Wormdingler boards.
>>>>>
>>>>> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
>>>>> Cc: Douglas Anderson <dianders@chromium.org>
>>>>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>>>>> ---
>>>>>
>>>>> This relies on commit 5e41b01a7808 ("drm/panel: Add an API to allow drm
>>>>> to set orientation from panel") which is in drm-misc
>>>>>
>>>>>   drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 ++
>>>>>   1 file changed, 2 insertions(+)
>>>>
>>>> I don't personally have objections to this, but (to my understanding)
>>>> "the future" is that everyone should use panel_bridge. If we made the
>>>> move to panel_bridge today then we wouldn't need to do this. In
>>>> general I think panel_bridge would end up letting us delete a bunch of
>>>> code...
>>>>
>>>> See commit 4e5763f03e10 ("drm/bridge: ti-sn65dsi86: Wrap panel with
>>>> panel-bridge") for when this was done by ti-sn65dsi86.
>>>>
>>>> Then again, I spent a small amount of time looking into this and it's
>>>> definitely non-trivial. Still likely worthwhile, but not worth
>>>> blocking a tiny fix like this. It also should be fairly obvious that
>>>> we should delete this when we switch to panel_bridge.
>>>>
>>>> Thus:
>>>>
>>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>>>
>>>> I'll assume that we'll just snooze this commit until drm-misc-next
>>>> merges into a tree that msm-next is based on, which will probably be
>>>> the next -rc1. If desired and Acked I could land this in
>>>> drm-misc-next, but it's probably not worth it?
>>>
>>> if you want to land this patch via drm-misc, which might be the
>>> easier/faster route, then:
>>>
>>> Acked-by: Rob Clark <robdclark@gmail.com>
>>
>> As per discussion on IRC, I'm not going to apply this to drm-misc-next.
>>
>> Given where we are in the cycle landing in drm-misc-next means it
>> won't be in mainline for a couple versions and I suspect that'll cause
>> merge conflicts with Dmitry's series [1]. ...and, of course, if
>> Dmitry's series lands then we don't even need ${SUBJECT} patch...
>>
>> So I think the plan is:
>>
>> 1. Snooze waiting for the next -rc1 since
>> drm_connector_set_orientation_from_panel() won't be in mainline until
>> then.
>>
>> 2. If Dmitry's series looks like a long way off, we could land
>> ${SUBJECT} patch in msm-next as a stopgap fix.
>>
>>
>> [1] https://lore.kernel.org/r/20220711094320.368062-5-dmitry.baryshkov@linaro.org/
> 
> Just checking up. What's the latest thinking here? Do we want to land
> Stephen's change as a stopgap?
> drm_connector_set_orientation_from_panel() is available in v6.0-rc1.
> 
> -Doug

As per todays discussion with Rob on IRC, we will start preparing the 
tree for the next release. So lets drop this one and take the panel 
bridge change instead since my comments on that were minor and can also 
be addressed in a follow up change, will take it up and send it over to 
Rob with some other changes.

Thanks

Abhinav

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/msm/dsi: Set panel orientation when directly connected
  2022-07-06 19:14 [PATCH] drm/msm/dsi: Set panel orientation when directly connected Stephen Boyd
  2022-07-07 21:11 ` Abhinav Kumar
  2022-07-08 15:25 ` Doug Anderson
@ 2022-09-01  8:42 ` Dmitry Baryshkov
  2 siblings, 0 replies; 14+ messages in thread
From: Dmitry Baryshkov @ 2022-09-01  8:42 UTC (permalink / raw)
  To: Stephen Boyd, Rob Clark, Abhinav Kumar
  Cc: linux-kernel, patches, Sean Paul, dri-devel, freedreno,
	Hsin-Yi Wang, Douglas Anderson

On 06/07/2022 22:14, Stephen Boyd wrote:
> Set the panel orientation in drm when the panel is directly connected,
> i.e. we're not using an external bridge. The external bridge case is
> already handled by the panel bridge code, so we only update the path we
> take when the panel is directly connected/internal. This silences a
> warning splat coming from __drm_mode_object_add() on Wormdingler boards.
> 
> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
> 
> This relies on commit 5e41b01a7808 ("drm/panel: Add an API to allow drm
> to set orientation from panel") which is in drm-misc

With the msm/dsi being switched to DRM_PANEL_BRIDGE this is no longer 
relevant. Dropping this from the queue.

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-09-01  8:42 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-06 19:14 [PATCH] drm/msm/dsi: Set panel orientation when directly connected Stephen Boyd
2022-07-07 21:11 ` Abhinav Kumar
2022-07-07 21:21   ` Stephen Boyd
2022-07-07 21:25     ` [Freedreno] " Abhinav Kumar
2022-07-08 15:25 ` Doug Anderson
2022-07-08 16:00   ` Abhinav Kumar
2022-07-08 19:42     ` Abhinav Kumar
2022-07-08 20:58       ` Dmitry Baryshkov
2022-07-08 21:17         ` Abhinav Kumar
2022-07-20 20:46   ` Rob Clark
2022-07-20 22:42     ` Doug Anderson
2022-08-17 20:48       ` Doug Anderson
2022-08-18 23:11         ` [Freedreno] " Abhinav Kumar
2022-09-01  8:42 ` Dmitry Baryshkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).