Re: [PATCH 3/5] drm/i915: Clarify sprite plane function names (v3)

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Dec 23, 2014 at 12:08:29PM +0200, Ander Conselvan de Oliveira wrote:
> On 12/16/2014 02:23 AM, Matt Roper wrote:
> >A few of the sprite-related function names in i915 are very similar
> >(e.g., intel_enable_planes() vs intel_crtc_enable_planes()) and don't
> >make it clear whether they only operate on sprite planes, or whether
> >they also apply to all universal plane types.  Rename a few functions to
> >be more consistent with our function naming for primary/cursor planes or
> >to clarify that they apply specifically to sprite planes:
> >
> >  - s/intel_disable_planes/intel_disable_sprite_planes/
> >  - s/intel_enable_planes/intel_enable_sprite_planes/
> >
> >Also, drop the sprite-specific intel_destroy_plane() and just use
> >the type-agnostic intel_plane_destroy() function.  The extra 'disable'
> >call that intel_destroy_plane() did is unnecessary since the plane will
> >already be disabled due to framebuffer destruction by the point it gets
> >called.
> >
> >v2: Earlier consolidation patches have reduced the number of functions
> >     we need to rename here.
> >
> >v3: Also rename intel_plane_funcs vtable to intel_sprite_plane_funcs
> >     for consistency with primary/cursor.  (Ander)
> >
> >Reviewed-by(v1): Bob Paauwe <bob.j.paauwe@intel.com>
> >Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_display.c | 10 +++++-----
> >  drivers/gpu/drm/i915/intel_drv.h     |  1 +
> >  drivers/gpu/drm/i915/intel_sprite.c  | 14 +++-----------
> >  3 files changed, 9 insertions(+), 16 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >index ce552d1..030cf93 100644
> >--- a/drivers/gpu/drm/i915/intel_display.c
> >+++ b/drivers/gpu/drm/i915/intel_display.c
> >@@ -4037,7 +4037,7 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
> >  	}
> >  }
> >
> >-static void intel_enable_planes(struct drm_crtc *crtc)
> >+static void intel_enable_sprite_planes(struct drm_crtc *crtc)
> >  {
> >  	struct drm_device *dev = crtc->dev;
> >  	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> >@@ -4051,7 +4051,7 @@ static void intel_enable_planes(struct drm_crtc *crtc)
> >  	}
> >  }
> >
> >-static void intel_disable_planes(struct drm_crtc *crtc)
> >+static void intel_disable_sprite_planes(struct drm_crtc *crtc)
> >  {
> >  	struct drm_device *dev = crtc->dev;
> >  	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> >@@ -4195,7 +4195,7 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc)
> >  	int pipe = intel_crtc->pipe;
> >
> >  	intel_enable_primary_hw_plane(crtc->primary, crtc);
> >-	intel_enable_planes(crtc);
> >+	intel_enable_sprite_planes(crtc);
> >  	intel_crtc_update_cursor(crtc, true);
> >  	intel_crtc_dpms_overlay(intel_crtc, true);
> >
> >@@ -4230,7 +4230,7 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
> >
> >  	intel_crtc_dpms_overlay(intel_crtc, false);
> >  	intel_crtc_update_cursor(crtc, false);
> >-	intel_disable_planes(crtc);
> >+	intel_disable_sprite_planes(crtc);
> >  	intel_disable_primary_hw_plane(crtc->primary, crtc);
> >
> >  	/*
> >@@ -12008,7 +12008,7 @@ intel_disable_plane(struct drm_plane *plane)
> >  }
> >
> >  /* Common destruction function for both primary and cursor planes */
> >-static void intel_plane_destroy(struct drm_plane *plane)
> >+void intel_plane_destroy(struct drm_plane *plane)
> 
> Do we need kernel doc for this?

Not sure kerneldoc for all the plane stuff is worth it yet while
everything is in-flux. Probably better to extract the higher-level stuff
into a new file/regroup functions a bit once the dust has settled and then
kerneldoc everything.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx