On Tue, Mar 10, 2015 at 01:15:25PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > In preparation to movable/resizable primary planes pass the clipped > plane size to .update_primary_plane(). Personally I feel like it would make more sense to just completely kill off .update_primary_plane() now rather than trying to evolve it. We already have an intel_plane->update_plane() function pointer which is never set or called for non-sprites at the moment. We could unify the handling of low-level plane programming by just using that function pointer for primary planes as well. I wouldn't mind also seeing the name of that low-level entrypoint renamed to something like 'update_hw_plane' to avoid confusion with the drm_plane entrypoint... Matt > > Cc: Sonika Jindal <sonika.jindal@xxxxxxxxx> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_drv.h | 3 +- > drivers/gpu/drm/i915/intel_display.c | 63 ++++++++++++++++++++---------------- > 2 files changed, 38 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index b16c0a7..e99eef0 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -578,7 +578,8 @@ struct drm_i915_display_funcs { > uint32_t flags); > void (*update_primary_plane)(struct drm_crtc *crtc, > struct drm_framebuffer *fb, > - int x, int y); > + int x, int y, > + int crtc_w, int crtc_h); > void (*hpd_irq_setup)(struct drm_device *dev); > /* clock updates for mode set */ > /* cursor updates */ > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index fdc83f1..1a789f0 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2482,7 +2482,8 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc, > > static void i9xx_update_primary_plane(struct drm_crtc *crtc, > struct drm_framebuffer *fb, > - int x, int y) > + int x, int y, > + int crtc_w, int crtc_h) > { > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -2517,20 +2518,16 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, > if (INTEL_INFO(dev)->gen < 4) { > if (intel_crtc->pipe == PIPE_B) > dspcntr |= DISPPLANE_SEL_PIPE_B; > - > - /* pipesrc and dspsize control the size that is scaled from, > - * which should always be the user's requested size. > - */ > - I915_WRITE(DSPSIZE(plane), > - ((intel_crtc->config->pipe_src_h - 1) << 16) | > - (intel_crtc->config->pipe_src_w - 1)); > + I915_WRITE(DSPSIZE(plane), ((crtc_h - 1) << 16) | (crtc_w - 1)); > I915_WRITE(DSPPOS(plane), 0); > } else if (IS_CHERRYVIEW(dev) && plane == PLANE_B) { > - I915_WRITE(PRIMSIZE(plane), > - ((intel_crtc->config->pipe_src_h - 1) << 16) | > - (intel_crtc->config->pipe_src_w - 1)); > + I915_WRITE(PRIMSIZE(plane), ((crtc_h - 1) << 16) | (crtc_w - 1)); > I915_WRITE(PRIMPOS(plane), 0); > I915_WRITE(PRIMCNSTALPHA(plane), 0); > + } else { > + WARN_ONCE(crtc_w != intel_crtc->config->pipe_src_w || > + crtc_h != intel_crtc->config->pipe_src_h, > + "primary plane size doesn't match pipe size\n"); > } > > switch (fb->pixel_format) { > @@ -2586,14 +2583,14 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, > if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) { > dspcntr |= DISPPLANE_ROTATE_180; > > - x += (intel_crtc->config->pipe_src_w - 1); > - y += (intel_crtc->config->pipe_src_h - 1); > + x += (crtc_w - 1); > + y += (crtc_h - 1); > > /* Finding the last pixel of the last line of the display > data and adding to linear_offset*/ > linear_offset += > - (intel_crtc->config->pipe_src_h - 1) * fb->pitches[0] + > - (intel_crtc->config->pipe_src_w - 1) * pixel_size; > + (crtc_h - 1) * fb->pitches[0] + > + (crtc_w - 1) * pixel_size; > } > > I915_WRITE(reg, dspcntr); > @@ -2611,7 +2608,8 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, > > static void ironlake_update_primary_plane(struct drm_crtc *crtc, > struct drm_framebuffer *fb, > - int x, int y) > + int x, int y, > + int crtc_w, int crtc_h) > { > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -2643,6 +2641,10 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc, > if (IS_HASWELL(dev) || IS_BROADWELL(dev)) > dspcntr |= DISPPLANE_PIPE_CSC_ENABLE; > > + WARN_ONCE(crtc_w != intel_crtc->config->pipe_src_w || > + crtc_h != intel_crtc->config->pipe_src_h, > + "primary plane size doesn't match pipe size\n"); > + > switch (fb->pixel_format) { > case DRM_FORMAT_C8: > dspcntr |= DISPPLANE_8BPP; > @@ -2686,14 +2688,14 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc, > dspcntr |= DISPPLANE_ROTATE_180; > > if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) { > - x += (intel_crtc->config->pipe_src_w - 1); > - y += (intel_crtc->config->pipe_src_h - 1); > + x += (crtc_w - 1); > + y += (crtc_h - 1); > > /* Finding the last pixel of the last line of the display > data and adding to linear_offset*/ > linear_offset += > - (intel_crtc->config->pipe_src_h - 1) * fb->pitches[0] + > - (intel_crtc->config->pipe_src_w - 1) * pixel_size; > + (crtc_h - 1) * fb->pitches[0] + > + (crtc_w - 1) * pixel_size; > } > } > > @@ -2747,7 +2749,8 @@ u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier, > > static void skylake_update_primary_plane(struct drm_crtc *crtc, > struct drm_framebuffer *fb, > - int x, int y) > + int x, int y, > + int crtc_w, int crtc_h) > { > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -2826,9 +2829,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > > I915_WRITE(PLANE_POS(pipe, 0), 0); > I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x); > - I915_WRITE(PLANE_SIZE(pipe, 0), > - (intel_crtc->config->pipe_src_h - 1) << 16 | > - (intel_crtc->config->pipe_src_w - 1)); > + I915_WRITE(PLANE_SIZE(pipe, 0), ((crtc_h - 1) << 16) | (crtc_w - 1)); > I915_WRITE(PLANE_STRIDE(pipe, 0), fb->pitches[0] / stride_div); > I915_WRITE(PLANE_SURF(pipe, 0), i915_gem_obj_ggtt_offset(obj)); > > @@ -2845,12 +2846,16 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb, > { > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > if (dev_priv->display.disable_fbc) > dev_priv->display.disable_fbc(dev); > > + /* FIXME: this will go badly if the fb isn't big enough */ > to_intel_crtc(crtc)->primary_enabled = true; > - dev_priv->display.update_primary_plane(crtc, fb, x, y); > + dev_priv->display.update_primary_plane(crtc, fb, x, y, > + intel_crtc->config->pipe_src_w, > + intel_crtc->config->pipe_src_h); > > return 0; > } > @@ -2885,7 +2890,9 @@ static void intel_update_primary_planes(struct drm_device *dev) > dev_priv->display.update_primary_plane(crtc, > state->base.fb, > state->src.x1 >> 16, > - state->src.y1 >> 16); > + state->src.y1 >> 16, > + drm_rect_width(&state->dst), > + drm_rect_height(&state->dst)); > } > > drm_modeset_unlock(&crtc->mutex); > @@ -12019,7 +12026,9 @@ intel_commit_primary_plane(struct drm_plane *plane, > dev_priv->display.update_primary_plane(crtc, > state->base.fb, > state->src.x1 >> 16, > - state->src.y1 >> 16); > + state->src.y1 >> 16, > + drm_rect_width(&state->dst), > + drm_rect_height(&state->dst)); > } > } > > -- > 2.0.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx