diff mbox series

[v1,12/12] drm/todo: Add bridge related todo items

Message ID 20220717175801.78668-5-sam@ravnborg.org (mailing list archive)
State Handled Elsewhere
Headers show
Series drm bridge updates | expand

Commit Message

Sam Ravnborg July 17, 2022, 5:58 p.m. UTC
Add todo in the hope someone will help updating the bridge drivers.

v2:
  - Updated descriptions in todo.rst

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Acked-by: Maxime Ripard <mripard@kernel.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 Documentation/gpu/todo.rst | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Dave Stevenson July 18, 2022, 10:27 a.m. UTC | #1
Hi Sam

On Sun, 17 Jul 2022 at 18:58, Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Add todo in the hope someone will help updating the bridge drivers.
>
> v2:
>   - Updated descriptions in todo.rst
>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Acked-by: Maxime Ripard <mripard@kernel.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> ---
>  Documentation/gpu/todo.rst | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 10bfb50908d1..fbcc232e0bc1 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -480,6 +480,26 @@ Contact: Thomas Zimmermann <tzimmermann@suse.de>
>
>  Level: Starter
>
> +Drop use of deprecated operations in bridge drivers
> +--------------------------------------------------
> +
> +&struct drm_bridge_funcs contains a number of deprecated operations
> +which can be replaced by the atomic variants.
> +
> +The following is more or less 1:1 replacements with the arguments
> +and names adjusted:
> +* pre_enable => atomic_pre_enable
> +* enable => atomic_enable
> +* disable => atomic_disable
> +* post_disable => atomic_post_disable
> +
> +* mode_set is no longer required and the implementation shall be merged
> +  with atomic_enable.

The dw-mipi-dsi and msm DSI host controller bridge drivers are
currently relying on mode_set as a convenient hook to power up the DSI
host prior to pre_enable of the bridge chain. Removing it is therefore
going to break those.

There is a proposed mechanism to allow for the removal of this hack
[1], but it's still waiting on R-B tags and/or discussion from bridge
maintainers (gentle nudge as you are one of those maintainers).

And do you mean merge with atomic_enable, or merge with
atomic_pre_enable? atomic_pre_enable would be more applicable for
almost all the bridges I'm aware of as they want to be configured
before video starts.

Cheers,
  Dave

[1] https://lists.freedesktop.org/archives/dri-devel/2022-March/345466.html

> +Contact: bridge maintainers, Sam Ravnborg <sam@ravnborg.org>,
> +         Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +
> +Level: Beginner or intermediate (depending on the driver)
>
>  Core refactorings
>  =================
> --
> 2.34.1
>
Sam Ravnborg July 18, 2022, 6 p.m. UTC | #2
Hi Dave,

On Mon, Jul 18, 2022 at 11:27:37AM +0100, Dave Stevenson wrote:
> Hi Sam
> 
> On Sun, 17 Jul 2022 at 18:58, Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > Add todo in the hope someone will help updating the bridge drivers.
> >
> > v2:
> >   - Updated descriptions in todo.rst
> >
> > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > Acked-by: Maxime Ripard <mripard@kernel.org>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > ---
> >  Documentation/gpu/todo.rst | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> > index 10bfb50908d1..fbcc232e0bc1 100644
> > --- a/Documentation/gpu/todo.rst
> > +++ b/Documentation/gpu/todo.rst
> > @@ -480,6 +480,26 @@ Contact: Thomas Zimmermann <tzimmermann@suse.de>
> >
> >  Level: Starter
> >
> > +Drop use of deprecated operations in bridge drivers
> > +--------------------------------------------------
> > +
> > +&struct drm_bridge_funcs contains a number of deprecated operations
> > +which can be replaced by the atomic variants.
> > +
> > +The following is more or less 1:1 replacements with the arguments
> > +and names adjusted:
> > +* pre_enable => atomic_pre_enable
> > +* enable => atomic_enable
> > +* disable => atomic_disable
> > +* post_disable => atomic_post_disable
> > +
> > +* mode_set is no longer required and the implementation shall be merged
> > +  with atomic_enable.
> 
> The dw-mipi-dsi and msm DSI host controller bridge drivers are
> currently relying on mode_set as a convenient hook to power up the DSI
> host prior to pre_enable of the bridge chain. Removing it is therefore
> going to break those.
> 
> There is a proposed mechanism to allow for the removal of this hack
> [1], but it's still waiting on R-B tags and/or discussion from bridge
> maintainers (gentle nudge as you are one of those maintainers).
I have over time gained some knowledge of how bridges works and have
applied a few patches - but the maintainer role belongs to others.
I just try to help a bit.

I will review the referenced series - could you then in return
review this series?
> 
> And do you mean merge with atomic_enable, or merge with
> atomic_pre_enable? atomic_pre_enable would be more applicable for
> almost all the bridges I'm aware of as they want to be configured
> before video starts.
Thanks, yes you are right. I will update the text to refer to
pre_enable as this is often the right choice. Looking at how many
bridges who implements mode_set, or are not yet atomic, this will
take a while before we can drop it.

	Sam
Dave Stevenson July 19, 2022, 10:47 a.m. UTC | #3
Hi Sam

On Mon, 18 Jul 2022 at 19:00, Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Dave,
>
> On Mon, Jul 18, 2022 at 11:27:37AM +0100, Dave Stevenson wrote:
> > Hi Sam
> >
> > On Sun, 17 Jul 2022 at 18:58, Sam Ravnborg <sam@ravnborg.org> wrote:
> > >
> > > Add todo in the hope someone will help updating the bridge drivers.
> > >
> > > v2:
> > >   - Updated descriptions in todo.rst
> > >
> > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > > Acked-by: Maxime Ripard <mripard@kernel.org>
> > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Maxime Ripard <mripard@kernel.org>
> > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > Cc: David Airlie <airlied@linux.ie>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > ---
> > >  Documentation/gpu/todo.rst | 20 ++++++++++++++++++++
> > >  1 file changed, 20 insertions(+)
> > >
> > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> > > index 10bfb50908d1..fbcc232e0bc1 100644
> > > --- a/Documentation/gpu/todo.rst
> > > +++ b/Documentation/gpu/todo.rst
> > > @@ -480,6 +480,26 @@ Contact: Thomas Zimmermann <tzimmermann@suse.de>
> > >
> > >  Level: Starter
> > >
> > > +Drop use of deprecated operations in bridge drivers
> > > +--------------------------------------------------
> > > +
> > > +&struct drm_bridge_funcs contains a number of deprecated operations
> > > +which can be replaced by the atomic variants.
> > > +
> > > +The following is more or less 1:1 replacements with the arguments
> > > +and names adjusted:
> > > +* pre_enable => atomic_pre_enable
> > > +* enable => atomic_enable
> > > +* disable => atomic_disable
> > > +* post_disable => atomic_post_disable
> > > +
> > > +* mode_set is no longer required and the implementation shall be merged
> > > +  with atomic_enable.
> >
> > The dw-mipi-dsi and msm DSI host controller bridge drivers are
> > currently relying on mode_set as a convenient hook to power up the DSI
> > host prior to pre_enable of the bridge chain. Removing it is therefore
> > going to break those.
> >
> > There is a proposed mechanism to allow for the removal of this hack
> > [1], but it's still waiting on R-B tags and/or discussion from bridge
> > maintainers (gentle nudge as you are one of those maintainers).
>
> I have over time gained some knowledge of how bridges works and have
> applied a few patches - but the maintainer role belongs to others.
> I just try to help a bit.

Apologies, I'd misread the text in this patch
+Contact: bridge maintainers, Sam Ravnborg <sam@ravnborg.org>,
+         Laurent Pinchart <laurent.pinchart@ideasonboard.com>
as being that you and Laurent were the bridge maintainers. Colon
instead of comma :-/

> I will review the referenced series - could you then in return
> review this series?

Sure, these look to be within my levels of knowledge.

> >
> > And do you mean merge with atomic_enable, or merge with
> > atomic_pre_enable? atomic_pre_enable would be more applicable for
> > almost all the bridges I'm aware of as they want to be configured
> > before video starts.
> Thanks, yes you are right. I will update the text to refer to
> pre_enable as this is often the right choice. Looking at how many
> bridges who implements mode_set, or are not yet atomic, this will
> take a while before we can drop it.

Thanks. That makes more logical sense to me for the majority of cases.
As to timescales, it always takes longer than ideal to migrate older drivers.

Thanks
  Dave
diff mbox series

Patch

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 10bfb50908d1..fbcc232e0bc1 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -480,6 +480,26 @@  Contact: Thomas Zimmermann <tzimmermann@suse.de>
 
 Level: Starter
 
+Drop use of deprecated operations in bridge drivers
+--------------------------------------------------
+
+&struct drm_bridge_funcs contains a number of deprecated operations
+which can be replaced by the atomic variants.
+
+The following is more or less 1:1 replacements with the arguments
+and names adjusted:
+* pre_enable => atomic_pre_enable
+* enable => atomic_enable
+* disable => atomic_disable
+* post_disable => atomic_post_disable
+
+* mode_set is no longer required and the implementation shall be merged
+  with atomic_enable.
+
+Contact: bridge maintainers, Sam Ravnborg <sam@ravnborg.org>,
+         Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+
+Level: Beginner or intermediate (depending on the driver)
 
 Core refactorings
 =================