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.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
