[2/3] drm: Remove references to removed transitional helpers
Commit Message
The transitional helpers were removed a long time ago, but some
references stuck. Remove them.
Fixes: 21ebe615c16994f3 ("drm: Remove transitional helpers")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
It doesn't look like the drm_encoder_helper_funcs were ever used by the
transitional plane helpers?
---
drivers/gpu/drm/drm_plane_helper.c | 8 ----
include/drm/drm_crtc.h | 5 ---
include/drm/drm_modeset_helper_vtables.h | 48 +++++++++++-------------
3 files changed, 21 insertions(+), 40 deletions(-)
Comments
Hi Geert,
Thank you for the patch.
On Fri, Jun 02, 2023 at 11:11:35AM +0200, Geert Uytterhoeven wrote:
> The transitional helpers were removed a long time ago, but some
> references stuck. Remove them.
>
> Fixes: 21ebe615c16994f3 ("drm: Remove transitional helpers")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> It doesn't look like the drm_encoder_helper_funcs were ever used by the
> transitional plane helpers?
> ---
> drivers/gpu/drm/drm_plane_helper.c | 8 ----
> include/drm/drm_crtc.h | 5 ---
> include/drm/drm_modeset_helper_vtables.h | 48 +++++++++++-------------
> 3 files changed, 21 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> index c91e454eba097942..be45bdb58d849653 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -51,14 +51,6 @@
> * planes, and newly merged drivers must not rely upon these transitional
> * helpers.
> *
The first paragraph starts with "This helper library has two parts.". As
you're dropping the mention of the second part, I think you should
rework the first paragraph too.
> - * The second part also implements transitional helpers which allow drivers to
> - * gradually switch to the atomic helper infrastructure for plane updates. Once
> - * that switch is complete drivers shouldn't use these any longer, instead using
> - * the proper legacy implementations for update and disable plane hooks provided
> - * by the atomic helpers.
> - *
> - * Again drivers are strongly urged to switch to the new interfaces.
> - *
> * The plane helpers share the function table structures with other helpers,
> * specifically also the atomic helpers. See &struct drm_plane_helper_funcs for
> * the details.
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 8e1cbc75143ef216..8b48a1974da3143c 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -77,11 +77,6 @@ struct drm_plane_helper_funcs;
> * intended to indicate whether a full modeset is needed, rather than strictly
> * describing what has changed in a commit. See also:
> * drm_atomic_crtc_needs_modeset()
> - *
> - * WARNING: Transitional helpers (like drm_helper_crtc_mode_set() or
> - * drm_helper_crtc_mode_set_base()) do not maintain many of the derived control
> - * state like @plane_mask so drivers not converted over to atomic helpers should
> - * not rely on these being accurate!
> */
> struct drm_crtc_state {
> /** @crtc: backpointer to the CRTC */
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index 965faf082a6d1acb..e3c3ac615909474b 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -59,8 +59,8 @@ enum mode_set_atomic {
> /**
> * struct drm_crtc_helper_funcs - helper operations for CRTCs
> *
> - * These hooks are used by the legacy CRTC helpers, the transitional plane
> - * helpers and the new atomic modesetting helpers.
> + * These hooks are used by the legacy CRTC helpers and the new atomic
> + * modesetting helpers.
> */
> struct drm_crtc_helper_funcs {
> /**
> @@ -216,9 +216,7 @@ struct drm_crtc_helper_funcs {
> *
> * This callback is used to update the display mode of a CRTC without
> * changing anything of the primary plane configuration. This fits the
> - * requirement of atomic and hence is used by the atomic helpers. It is
> - * also used by the transitional plane helpers to implement a
> - * @mode_set hook in drm_helper_crtc_mode_set().
> + * requirement of atomic and hence is used by the atomic helpers.
> *
> * Note that the display pipe is completely off when this function is
> * called. Atomic drivers which need hardware to be running before they
> @@ -333,8 +331,8 @@ struct drm_crtc_helper_funcs {
> * all updated. Again the recommendation is to just call check helpers
> * until a maximal configuration is reached.
> *
> - * This callback is used by the atomic modeset helpers and by the
> - * transitional plane helpers, but it is optional.
> + * This callback is used by the atomic modeset helpers, but it is
> + * optional.
> *
> * NOTE:
> *
> @@ -373,8 +371,8 @@ struct drm_crtc_helper_funcs {
> * has picked. See drm_atomic_helper_commit_planes() for a discussion of
> * the tradeoffs and variants of plane commit helpers.
> *
> - * This callback is used by the atomic modeset helpers and by the
> - * transitional plane helpers, but it is optional.
> + * This callback is used by the atomic modeset helpers, but it is
> + * optional.
> */
> void (*atomic_begin)(struct drm_crtc *crtc,
> struct drm_atomic_state *state);
> @@ -397,8 +395,8 @@ struct drm_crtc_helper_funcs {
> * has picked. See drm_atomic_helper_commit_planes() for a discussion of
> * the tradeoffs and variants of plane commit helpers.
> *
> - * This callback is used by the atomic modeset helpers and by the
> - * transitional plane helpers, but it is optional.
> + * This callback is used by the atomic modeset helpers, but it is
> + * optional.
> */
> void (*atomic_flush)(struct drm_crtc *crtc,
> struct drm_atomic_state *state);
> @@ -507,8 +505,8 @@ static inline void drm_crtc_helper_add(struct drm_crtc *crtc,
> /**
> * struct drm_encoder_helper_funcs - helper operations for encoders
> *
> - * These hooks are used by the legacy CRTC helpers, the transitional plane
> - * helpers and the new atomic modesetting helpers.
> + * These hooks are used by the legacy CRTC helpers and the new atomic
> + * modesetting helpers.
> */
> struct drm_encoder_helper_funcs {
> /**
> @@ -1185,8 +1183,7 @@ static inline void drm_connector_helper_add(struct drm_connector *connector,
> /**
> * struct drm_plane_helper_funcs - helper operations for planes
> *
> - * These functions are used by the atomic helpers and by the transitional plane
> - * helpers.
> + * These functions are used by the atomic helpers.
> */
> struct drm_plane_helper_funcs {
> /**
> @@ -1221,9 +1218,8 @@ struct drm_plane_helper_funcs {
> * The helpers will call @cleanup_fb with matching arguments for every
> * successful call to this hook.
> *
> - * This callback is used by the atomic modeset helpers and by the
> - * transitional plane helpers, but it is optional. See @begin_fb_access
> - * for preparing per-commit resources.
> + * This callback is used by the atomic modeset helpers, but it is
> + * optional. See @begin_fb_access for preparing per-commit resources.
> *
> * RETURNS:
> *
> @@ -1240,8 +1236,8 @@ struct drm_plane_helper_funcs {
> * This hook is called to clean up any resources allocated for the given
> * framebuffer and plane configuration in @prepare_fb.
> *
> - * This callback is used by the atomic modeset helpers and by the
> - * transitional plane helpers, but it is optional.
> + * This callback is used by the atomic modeset helpers, but it is
> + * optional.
> */
> void (*cleanup_fb)(struct drm_plane *plane,
> struct drm_plane_state *old_state);
> @@ -1295,8 +1291,8 @@ struct drm_plane_helper_funcs {
> * all updated. Again the recommendation is to just call check helpers
> * until a maximal configuration is reached.
> *
> - * This callback is used by the atomic modeset helpers and by the
> - * transitional plane helpers, but it is optional.
> + * This callback is used by the atomic modeset helpers, but it is
> + * optional.
> *
> * NOTE:
> *
> @@ -1326,8 +1322,7 @@ struct drm_plane_helper_funcs {
> * has picked. See drm_atomic_helper_commit_planes() for a discussion of
> * the tradeoffs and variants of plane commit helpers.
> *
> - * This callback is used by the atomic modeset helpers and by the
> - * transitional plane helpers, but it is optional.
> + * This callback is used by the atomic modeset helpers, but it is optional.
> */
> void (*atomic_update)(struct drm_plane *plane,
> struct drm_atomic_state *state);
> @@ -1376,9 +1371,8 @@ struct drm_plane_helper_funcs {
> * has picked. See drm_atomic_helper_commit_planes() for a discussion of
> * the tradeoffs and variants of plane commit helpers.
> *
> - * This callback is used by the atomic modeset helpers and by the
> - * transitional plane helpers, but it is optional. It's intended to
> - * reverse the effects of @atomic_enable.
> + * This callback is used by the atomic modeset helpers, but it is
> + * optional. It's intended to reverse the effects of @atomic_enable.
> */
> void (*atomic_disable)(struct drm_plane *plane,
> struct drm_atomic_state *state);
Hi Laurent,
On Fri, Jun 2, 2023 at 1:05 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Fri, Jun 02, 2023 at 11:11:35AM +0200, Geert Uytterhoeven wrote:
> > The transitional helpers were removed a long time ago, but some
> > references stuck. Remove them.
> >
> > Fixes: 21ebe615c16994f3 ("drm: Remove transitional helpers")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > --- a/drivers/gpu/drm/drm_plane_helper.c
> > +++ b/drivers/gpu/drm/drm_plane_helper.c
> > @@ -51,14 +51,6 @@
> > * planes, and newly merged drivers must not rely upon these transitional
> > * helpers.
> > *
>
> The first paragraph starts with "This helper library has two parts.". As
> you're dropping the mention of the second part, I think you should
> rework the first paragraph too.
That was my initial thought, too.
However, the code still has a second part, not related to the topic of
the first part (primary plane support).
>
> > - * The second part also implements transitional helpers which allow drivers to
> > - * gradually switch to the atomic helper infrastructure for plane updates. Once
> > - * that switch is complete drivers shouldn't use these any longer, instead using
> > - * the proper legacy implementations for update and disable plane hooks provided
> > - * by the atomic helpers.
> > - *
> > - * Again drivers are strongly urged to switch to the new interfaces.
> > - *
> > * The plane helpers share the function table structures with other helpers,
> > * specifically also the atomic helpers. See &struct drm_plane_helper_funcs for
> > * the details.
Gr{oetje,eeting}s,
Geert
Hi Geert,
On Fri, Jun 02, 2023 at 01:17:58PM +0200, Geert Uytterhoeven wrote:
> On Fri, Jun 2, 2023 at 1:05 PM Laurent Pinchart wrote:
> > On Fri, Jun 02, 2023 at 11:11:35AM +0200, Geert Uytterhoeven wrote:
> > > The transitional helpers were removed a long time ago, but some
> > > references stuck. Remove them.
> > >
> > > Fixes: 21ebe615c16994f3 ("drm: Remove transitional helpers")
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> > > --- a/drivers/gpu/drm/drm_plane_helper.c
> > > +++ b/drivers/gpu/drm/drm_plane_helper.c
> > > @@ -51,14 +51,6 @@
> > > * planes, and newly merged drivers must not rely upon these transitional
> > > * helpers.
> > > *
> >
> > The first paragraph starts with "This helper library has two parts.". As
> > you're dropping the mention of the second part, I think you should
> > rework the first paragraph too.
>
> That was my initial thought, too.
> However, the code still has a second part, not related to the topic of
> the first part (primary plane support).
How about mentioning that in the comment then ?
> > > - * The second part also implements transitional helpers which allow drivers to
> > > - * gradually switch to the atomic helper infrastructure for plane updates. Once
> > > - * that switch is complete drivers shouldn't use these any longer, instead using
> > > - * the proper legacy implementations for update and disable plane hooks provided
> > > - * by the atomic helpers.
> > > - *
> > > - * Again drivers are strongly urged to switch to the new interfaces.
> > > - *
> > > * The plane helpers share the function table structures with other helpers,
> > > * specifically also the atomic helpers. See &struct drm_plane_helper_funcs for
> > > * the details.
Hi Laurent,
On Fri, Jun 2, 2023 at 1:36 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Fri, Jun 02, 2023 at 01:17:58PM +0200, Geert Uytterhoeven wrote:
> > On Fri, Jun 2, 2023 at 1:05 PM Laurent Pinchart wrote:
> > > On Fri, Jun 02, 2023 at 11:11:35AM +0200, Geert Uytterhoeven wrote:
> > > > The transitional helpers were removed a long time ago, but some
> > > > references stuck. Remove them.
> > > >
> > > > Fixes: 21ebe615c16994f3 ("drm: Remove transitional helpers")
> > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > > > --- a/drivers/gpu/drm/drm_plane_helper.c
> > > > +++ b/drivers/gpu/drm/drm_plane_helper.c
> > > > @@ -51,14 +51,6 @@
> > > > * planes, and newly merged drivers must not rely upon these transitional
> > > > * helpers.
> > > > *
> > >
> > > The first paragraph starts with "This helper library has two parts.". As
> > > you're dropping the mention of the second part, I think you should
> > > rework the first paragraph too.
> >
> > That was my initial thought, too.
> > However, the code still has a second part, not related to the topic of
> > the first part (primary plane support).
>
> How about mentioning that in the comment then ?
Any suggestion about the wording?
The drm novice in me would write "The second part is not about
primary plane support" ;-)
Gr{oetje,eeting}s,
Geert
On Fri, Jun 2, 2023 at 1:17 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Fri, Jun 2, 2023 at 1:05 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Fri, Jun 02, 2023 at 11:11:35AM +0200, Geert Uytterhoeven wrote:
> > > The transitional helpers were removed a long time ago, but some
> > > references stuck. Remove them.
> > >
> > > Fixes: 21ebe615c16994f3 ("drm: Remove transitional helpers")
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> > > --- a/drivers/gpu/drm/drm_plane_helper.c
> > > +++ b/drivers/gpu/drm/drm_plane_helper.c
> > > @@ -51,14 +51,6 @@
> > > * planes, and newly merged drivers must not rely upon these transitional
> > > * helpers.
> > > *
> >
> > The first paragraph starts with "This helper library has two parts.". As
> > you're dropping the mention of the second part, I think you should
> > rework the first paragraph too.
>
> That was my initial thought, too.
> However, the code still has a second part, not related to the topic of
> the first part (primary plane support).
Upon deeper investigation, all of this is related to primary plane
support. Will update...
Gr{oetje,eeting}s,
Geert
@@ -51,14 +51,6 @@
* planes, and newly merged drivers must not rely upon these transitional
* helpers.
*
- * The second part also implements transitional helpers which allow drivers to
- * gradually switch to the atomic helper infrastructure for plane updates. Once
- * that switch is complete drivers shouldn't use these any longer, instead using
- * the proper legacy implementations for update and disable plane hooks provided
- * by the atomic helpers.
- *
- * Again drivers are strongly urged to switch to the new interfaces.
- *
* The plane helpers share the function table structures with other helpers,
* specifically also the atomic helpers. See &struct drm_plane_helper_funcs for
* the details.
@@ -77,11 +77,6 @@ struct drm_plane_helper_funcs;
* intended to indicate whether a full modeset is needed, rather than strictly
* describing what has changed in a commit. See also:
* drm_atomic_crtc_needs_modeset()
- *
- * WARNING: Transitional helpers (like drm_helper_crtc_mode_set() or
- * drm_helper_crtc_mode_set_base()) do not maintain many of the derived control
- * state like @plane_mask so drivers not converted over to atomic helpers should
- * not rely on these being accurate!
*/
struct drm_crtc_state {
/** @crtc: backpointer to the CRTC */
@@ -59,8 +59,8 @@ enum mode_set_atomic {
/**
* struct drm_crtc_helper_funcs - helper operations for CRTCs
*
- * These hooks are used by the legacy CRTC helpers, the transitional plane
- * helpers and the new atomic modesetting helpers.
+ * These hooks are used by the legacy CRTC helpers and the new atomic
+ * modesetting helpers.
*/
struct drm_crtc_helper_funcs {
/**
@@ -216,9 +216,7 @@ struct drm_crtc_helper_funcs {
*
* This callback is used to update the display mode of a CRTC without
* changing anything of the primary plane configuration. This fits the
- * requirement of atomic and hence is used by the atomic helpers. It is
- * also used by the transitional plane helpers to implement a
- * @mode_set hook in drm_helper_crtc_mode_set().
+ * requirement of atomic and hence is used by the atomic helpers.
*
* Note that the display pipe is completely off when this function is
* called. Atomic drivers which need hardware to be running before they
@@ -333,8 +331,8 @@ struct drm_crtc_helper_funcs {
* all updated. Again the recommendation is to just call check helpers
* until a maximal configuration is reached.
*
- * This callback is used by the atomic modeset helpers and by the
- * transitional plane helpers, but it is optional.
+ * This callback is used by the atomic modeset helpers, but it is
+ * optional.
*
* NOTE:
*
@@ -373,8 +371,8 @@ struct drm_crtc_helper_funcs {
* has picked. See drm_atomic_helper_commit_planes() for a discussion of
* the tradeoffs and variants of plane commit helpers.
*
- * This callback is used by the atomic modeset helpers and by the
- * transitional plane helpers, but it is optional.
+ * This callback is used by the atomic modeset helpers, but it is
+ * optional.
*/
void (*atomic_begin)(struct drm_crtc *crtc,
struct drm_atomic_state *state);
@@ -397,8 +395,8 @@ struct drm_crtc_helper_funcs {
* has picked. See drm_atomic_helper_commit_planes() for a discussion of
* the tradeoffs and variants of plane commit helpers.
*
- * This callback is used by the atomic modeset helpers and by the
- * transitional plane helpers, but it is optional.
+ * This callback is used by the atomic modeset helpers, but it is
+ * optional.
*/
void (*atomic_flush)(struct drm_crtc *crtc,
struct drm_atomic_state *state);
@@ -507,8 +505,8 @@ static inline void drm_crtc_helper_add(struct drm_crtc *crtc,
/**
* struct drm_encoder_helper_funcs - helper operations for encoders
*
- * These hooks are used by the legacy CRTC helpers, the transitional plane
- * helpers and the new atomic modesetting helpers.
+ * These hooks are used by the legacy CRTC helpers and the new atomic
+ * modesetting helpers.
*/
struct drm_encoder_helper_funcs {
/**
@@ -1185,8 +1183,7 @@ static inline void drm_connector_helper_add(struct drm_connector *connector,
/**
* struct drm_plane_helper_funcs - helper operations for planes
*
- * These functions are used by the atomic helpers and by the transitional plane
- * helpers.
+ * These functions are used by the atomic helpers.
*/
struct drm_plane_helper_funcs {
/**
@@ -1221,9 +1218,8 @@ struct drm_plane_helper_funcs {
* The helpers will call @cleanup_fb with matching arguments for every
* successful call to this hook.
*
- * This callback is used by the atomic modeset helpers and by the
- * transitional plane helpers, but it is optional. See @begin_fb_access
- * for preparing per-commit resources.
+ * This callback is used by the atomic modeset helpers, but it is
+ * optional. See @begin_fb_access for preparing per-commit resources.
*
* RETURNS:
*
@@ -1240,8 +1236,8 @@ struct drm_plane_helper_funcs {
* This hook is called to clean up any resources allocated for the given
* framebuffer and plane configuration in @prepare_fb.
*
- * This callback is used by the atomic modeset helpers and by the
- * transitional plane helpers, but it is optional.
+ * This callback is used by the atomic modeset helpers, but it is
+ * optional.
*/
void (*cleanup_fb)(struct drm_plane *plane,
struct drm_plane_state *old_state);
@@ -1295,8 +1291,8 @@ struct drm_plane_helper_funcs {
* all updated. Again the recommendation is to just call check helpers
* until a maximal configuration is reached.
*
- * This callback is used by the atomic modeset helpers and by the
- * transitional plane helpers, but it is optional.
+ * This callback is used by the atomic modeset helpers, but it is
+ * optional.
*
* NOTE:
*
@@ -1326,8 +1322,7 @@ struct drm_plane_helper_funcs {
* has picked. See drm_atomic_helper_commit_planes() for a discussion of
* the tradeoffs and variants of plane commit helpers.
*
- * This callback is used by the atomic modeset helpers and by the
- * transitional plane helpers, but it is optional.
+ * This callback is used by the atomic modeset helpers, but it is optional.
*/
void (*atomic_update)(struct drm_plane *plane,
struct drm_atomic_state *state);
@@ -1376,9 +1371,8 @@ struct drm_plane_helper_funcs {
* has picked. See drm_atomic_helper_commit_planes() for a discussion of
* the tradeoffs and variants of plane commit helpers.
*
- * This callback is used by the atomic modeset helpers and by the
- * transitional plane helpers, but it is optional. It's intended to
- * reverse the effects of @atomic_enable.
+ * This callback is used by the atomic modeset helpers, but it is
+ * optional. It's intended to reverse the effects of @atomic_enable.
*/
void (*atomic_disable)(struct drm_plane *plane,
struct drm_atomic_state *state);