[17/21] drm: Unexport drm_plane_helper_check_update
diff mbox series

Message ID 20181004202446.22905-18-daniel.vetter@ffwll.ch
State New
Headers show
Series
  • atomic helper cleanup, take 2
Related show

Commit Message

Daniel Vetter Oct. 4, 2018, 8:24 p.m. UTC
It's for legacy drivers only (atomic ones should use
drm_atomic_helper_check_plane_state() instead), and there's no users
left except the one in the primary plane helpers.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_plane_helper.c | 49 +++++++-----------------------
 include/drm/drm_plane_helper.h     | 11 -------
 2 files changed, 11 insertions(+), 49 deletions(-)

Comments

Sam Ravnborg Oct. 4, 2018, 9:03 p.m. UTC | #1
On Thu, Oct 04, 2018 at 10:24:42PM +0200, Daniel Vetter wrote:
> It's for legacy drivers only (atomic ones should use
> drm_atomic_helper_check_plane_state() instead), and there's no users
> left except the one in the primary plane helpers.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_plane_helper.c | 49 +++++++-----------------------
>  include/drm/drm_plane_helper.h     | 11 -------
>  2 files changed, 11 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> index 965286231600..33b3e6892787 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -100,43 +100,17 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
>  	return count;
>  }
>  
> -/**
> - * drm_plane_helper_check_update() - Check plane update for validity
> - * @plane: plane object to update
> - * @crtc: owning CRTC of owning plane
> - * @fb: framebuffer to flip onto plane
> - * @src: source coordinates in 16.16 fixed point
> - * @dst: integer destination coordinates
> - * @rotation: plane rotation
> - * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> - * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> - * @can_position: is it legal to position the plane such that it
> - *                doesn't cover the entire crtc?  This will generally
> - *                only be false for primary planes.
> - * @can_update_disabled: can the plane be updated while the crtc
> - *                       is disabled?
> - * @visible: output parameter indicating whether plane is still visible after
> - *           clipping
> - *
> - * Checks that a desired plane update is valid.  Drivers that provide
> - * their own plane handling rather than helper-provided implementations may
> - * still wish to call this function to avoid duplication of error checking
> - * code.
> - *
> - * RETURNS:
> - * Zero if update appears valid, error code on failure
> - */
> -int drm_plane_helper_check_update(struct drm_plane *plane,
> -				  struct drm_crtc *crtc,
> -				  struct drm_framebuffer *fb,
> -				  struct drm_rect *src,
> -				  struct drm_rect *dst,
> -				  unsigned int rotation,
> -				  int min_scale,
> -				  int max_scale,
> -				  bool can_position,
> -				  bool can_update_disabled,
> -				  bool *visible)
> +static int drm_plane_helper_check_update(struct drm_plane *plane,

This patch makes the function static, which does not render the comment useless.
It would make sense to keep the comment around, but maybe in a more dense
format without all paramters explained.

Maybe just a simple:
/*
 * Checks that a desired plane update is valid.
 * Returns 0 if update appears valid, error code on failure
 */

	Sam
Daniel Vetter Oct. 5, 2018, 6:13 a.m. UTC | #2
On Thu, Oct 4, 2018 at 11:03 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> On Thu, Oct 04, 2018 at 10:24:42PM +0200, Daniel Vetter wrote:
> > It's for legacy drivers only (atomic ones should use
> > drm_atomic_helper_check_plane_state() instead), and there's no users
> > left except the one in the primary plane helpers.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/drm_plane_helper.c | 49 +++++++-----------------------
> >  include/drm/drm_plane_helper.h     | 11 -------
> >  2 files changed, 11 insertions(+), 49 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> > index 965286231600..33b3e6892787 100644
> > --- a/drivers/gpu/drm/drm_plane_helper.c
> > +++ b/drivers/gpu/drm/drm_plane_helper.c
> > @@ -100,43 +100,17 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
> >       return count;
> >  }
> >
> > -/**
> > - * drm_plane_helper_check_update() - Check plane update for validity
> > - * @plane: plane object to update
> > - * @crtc: owning CRTC of owning plane
> > - * @fb: framebuffer to flip onto plane
> > - * @src: source coordinates in 16.16 fixed point
> > - * @dst: integer destination coordinates
> > - * @rotation: plane rotation
> > - * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> > - * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> > - * @can_position: is it legal to position the plane such that it
> > - *                doesn't cover the entire crtc?  This will generally
> > - *                only be false for primary planes.
> > - * @can_update_disabled: can the plane be updated while the crtc
> > - *                       is disabled?
> > - * @visible: output parameter indicating whether plane is still visible after
> > - *           clipping
> > - *
> > - * Checks that a desired plane update is valid.  Drivers that provide
> > - * their own plane handling rather than helper-provided implementations may
> > - * still wish to call this function to avoid duplication of error checking
> > - * code.
> > - *
> > - * RETURNS:
> > - * Zero if update appears valid, error code on failure
> > - */
> > -int drm_plane_helper_check_update(struct drm_plane *plane,
> > -                               struct drm_crtc *crtc,
> > -                               struct drm_framebuffer *fb,
> > -                               struct drm_rect *src,
> > -                               struct drm_rect *dst,
> > -                               unsigned int rotation,
> > -                               int min_scale,
> > -                               int max_scale,
> > -                               bool can_position,
> > -                               bool can_update_disabled,
> > -                               bool *visible)
> > +static int drm_plane_helper_check_update(struct drm_plane *plane,
>
> This patch makes the function static, which does not render the comment useless.
> It would make sense to keep the comment around, but maybe in a more dense
> format without all paramters explained.
>
> Maybe just a simple:
> /*
>  * Checks that a desired plane update is valid.
>  * Returns 0 if update appears valid, error code on failure
>  */

I felt like the function name sufficiently explains what it does. I do
occasionally keep the kerneldoc when making a function static, but
only when it explains something tricky. "People who work on drm core
code" is a different audience from "people who write drm drivers". If
we'd want comments like the above on all static functions in the drm
core, someone would need to do a _lot_ of typing :-)
-Daniel

Patch
diff mbox series

diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index 965286231600..33b3e6892787 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -100,43 +100,17 @@  static int get_connectors_for_crtc(struct drm_crtc *crtc,
 	return count;
 }
 
-/**
- * drm_plane_helper_check_update() - Check plane update for validity
- * @plane: plane object to update
- * @crtc: owning CRTC of owning plane
- * @fb: framebuffer to flip onto plane
- * @src: source coordinates in 16.16 fixed point
- * @dst: integer destination coordinates
- * @rotation: plane rotation
- * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
- * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
- * @can_position: is it legal to position the plane such that it
- *                doesn't cover the entire crtc?  This will generally
- *                only be false for primary planes.
- * @can_update_disabled: can the plane be updated while the crtc
- *                       is disabled?
- * @visible: output parameter indicating whether plane is still visible after
- *           clipping
- *
- * Checks that a desired plane update is valid.  Drivers that provide
- * their own plane handling rather than helper-provided implementations may
- * still wish to call this function to avoid duplication of error checking
- * code.
- *
- * RETURNS:
- * Zero if update appears valid, error code on failure
- */
-int drm_plane_helper_check_update(struct drm_plane *plane,
-				  struct drm_crtc *crtc,
-				  struct drm_framebuffer *fb,
-				  struct drm_rect *src,
-				  struct drm_rect *dst,
-				  unsigned int rotation,
-				  int min_scale,
-				  int max_scale,
-				  bool can_position,
-				  bool can_update_disabled,
-				  bool *visible)
+static int drm_plane_helper_check_update(struct drm_plane *plane,
+					 struct drm_crtc *crtc,
+					 struct drm_framebuffer *fb,
+					 struct drm_rect *src,
+					 struct drm_rect *dst,
+					 unsigned int rotation,
+					 int min_scale,
+					 int max_scale,
+					 bool can_position,
+					 bool can_update_disabled,
+					 bool *visible)
 {
 	struct drm_plane_state plane_state = {
 		.plane = plane,
@@ -173,7 +147,6 @@  int drm_plane_helper_check_update(struct drm_plane *plane,
 
 	return 0;
 }
-EXPORT_SYMBOL(drm_plane_helper_check_update);
 
 /**
  * drm_primary_helper_update() - Helper for primary plane update
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 1999781781c7..7bff930b8b10 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -38,17 +38,6 @@ 
  */
 #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
 
-int drm_plane_helper_check_update(struct drm_plane *plane,
-				  struct drm_crtc *crtc,
-				  struct drm_framebuffer *fb,
-				  struct drm_rect *src,
-				  struct drm_rect *dest,
-				  unsigned int rotation,
-				  int min_scale,
-				  int max_scale,
-				  bool can_position,
-				  bool can_update_disabled,
-				  bool *visible);
 int drm_primary_helper_update(struct drm_plane *plane,
 			      struct drm_crtc *crtc,
 			      struct drm_framebuffer *fb,