Re: [PATCH 11/22] drm/i915: Don't pass plane+plane_state to intel_pin_and_fence_fb_obj()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 15/10/15 12:17, Ville Syrjälä wrote:
On Thu, Oct 15, 2015 at 12:10:32PM +0100, Tvrtko Ursulin wrote:

Hi,

On 14/10/15 17:29, ville.syrjala@xxxxxxxxxxxxxxx wrote:
From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

intel_pin_and_fence_fb_obj() only needs the framebuffer, and the desird
rotation (to find the right GTT view for it), so no need to pass all
kinds of plane stuff.

Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
   drivers/gpu/drm/i915/intel_display.c | 39 ++++++++++++++++--------------------
   drivers/gpu/drm/i915/intel_drv.h     |  5 ++---
   drivers/gpu/drm/i915/intel_fbdev.c   |  2 +-
   3 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 85e1473..80e9f2e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2275,8 +2275,9 @@ intel_fb_align_height(struct drm_device *dev, unsigned int height,
   }

   static int
-intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb,
-			const struct drm_plane_state *plane_state)
+intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
+			const struct drm_framebuffer *fb,
+			unsigned int rotation)
   {
   	struct drm_i915_private *dev_priv = to_i915(fb->dev);
   	struct intel_rotation_info *info = &view->rotation_info;
@@ -2284,10 +2285,7 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb,

   	*view = i915_ggtt_view_normal;

-	if (!plane_state)
-		return 0;
-
-	if (!intel_rotation_90_or_270(plane_state->rotation))
+	if (!intel_rotation_90_or_270(rotation))
   		return 0;

   	*view = i915_ggtt_view_rotated;
@@ -2354,9 +2352,8 @@ static unsigned int intel_surf_alignment(const struct drm_i915_private *dev_priv
   }

   int
-intel_pin_and_fence_fb_obj(struct drm_plane *plane,
-			   struct drm_framebuffer *fb,
-			   const struct drm_plane_state *plane_state,
+intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
+			   unsigned int rotation,
   			   struct intel_engine_cs *pipelined,
   			   struct drm_i915_gem_request **pipelined_request)
   {

It feels like you are losing the benefit of cleaning this up by having
to pass in rotation anyway. So I think it makes more sense to keep
passing in plane_state and only get rid of the plane. Or vice-versa, not
really sure what is conceptually better. Possibly plane and then access
the state from it.

The only thing we basically need is "which vma do we want". But just
passing rotation directly looks nicer I think. The benefit really is
eliminating the ugly 'if (!plane_state)' mess caused by intel_fbdev.

We'll have to disagree there then because I find it really inelegant to express the knowledge of what exact information is needed when preparing the frame buffer for display into the function parameters.

It is conceptually much more elegant to say "this fb for this plane - do what is right".

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux