On Tue, Mar 10, 2015 at 10:10:40AM -0700, Matt Roper wrote:
> On Tue, Mar 10, 2015 at 01:15:25PM +0200, [email protected] wrote:
> > From: Ville Syrjälä <[email protected]>
> > 
> > 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 want to kill it off as well, but that means either killing off
set_base_atomic() or making it use the plane commit hook. I suppose we
could hand craft a suitable plane state for it and just commit that
without any checks or anything?

> 
> 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 <[email protected]>
> > Signed-off-by: Ville Syrjälä <[email protected]>
> > ---
> >  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
> > [email protected]
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to