On Thu, Nov 17, 2016 at 06:29:45PM -0200, Paulo Zanoni wrote:
> Em Ter, 2016-11-08 às 16:47 +0200, ville.syrj...@linux.intel.com
> escreveu:
> > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > 
> > Let's try not to abuse plane->plane for sprites on VLV/CHV and
> > instead
> > use plane->id. Since out watermark structures aren't entirely plane
> > type
> > agnostic (for now) and start indexing sprites from 0  we'll add a
> > small
> > helper to convert between the two bases.
> 
> I'm not sure I like this one. We've been using plane_id for the actual
> prim/spr/cur IDs, and now we use plane_id for something that's supposed
> to mean A/B/C. I really think we should have different words to refer
> to the two things. I think this goes back to Matt's renaming discussion
> we've been having in patch 2. Let's solve it there first before we
> revisit this patch. I'm still not sure what would be the ideal solution
> here, I'll spend some time thinking.

Replied to the wrong patch?

> 
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 73 ++++++++++++++++++++-----------
> > ----------
> >  1 file changed, 36 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index fd8cbc224b07..b1ad09e458ca 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -371,12 +371,15 @@ static const int pessimal_latency_ns = 5000;
> >  #define VLV_FIFO_START(dsparb, dsparb2, lo_shift, hi_shift) \
> >     ((((dsparb) >> (lo_shift)) & 0xff) | ((((dsparb2) >>
> > (hi_shift)) & 0x1) << 8))
> >  
> > -static int vlv_get_fifo_size(struct drm_i915_private *dev_priv,
> > -                         enum pipe pipe, int plane)
> > +static int vlv_get_fifo_size(struct intel_plane *plane)
> >  {
> > +   struct drm_i915_private *dev_priv = to_i915(plane-
> > >base.dev);
> >     int sprite0_start, sprite1_start, size;
> >  
> > -   switch (pipe) {
> > +   if (plane->id == PLANE_CURSOR)
> > +           return 63;
> > +
> > +   switch (plane->pipe) {
> >             uint32_t dsparb, dsparb2, dsparb3;
> >     case PIPE_A:
> >             dsparb = I915_READ(DSPARB);
> > @@ -400,24 +403,21 @@ static int vlv_get_fifo_size(struct
> > drm_i915_private *dev_priv,
> >             return 0;
> >     }
> >  
> > -   switch (plane) {
> > -   case 0:
> > +   switch (plane->id) {
> > +   case PLANE_PRIMARY:
> >             size = sprite0_start;
> >             break;
> > -   case 1:
> > +   case PLANE_SPRITE0:
> >             size = sprite1_start - sprite0_start;
> >             break;
> > -   case 2:
> > +   case PLANE_SPRITE1:
> >             size = 512 - 1 - sprite1_start;
> >             break;
> >     default:
> >             return 0;
> >     }
> >  
> > -   DRM_DEBUG_KMS("Pipe %c %s %c FIFO size: %d\n",
> > -                 pipe_name(pipe), plane == 0 ? "primary" :
> > "sprite",
> > -                 plane == 0 ? plane_name(pipe) :
> > sprite_name(pipe, plane - 1),
> > -                 size);
> > +   DRM_DEBUG_KMS("%s FIFO size: %d\n", plane->base.name, size);
> >  
> >     return size;
> >  }
> > @@ -1054,6 +1054,12 @@ static void vlv_compute_fifo(struct intel_crtc
> > *crtc)
> >     WARN_ON(fifo_left != 0);
> >  }
> >  
> > +/* FIXME kill me */
> > +static inline int vlv_sprite_id(enum plane_id plane_id)
> > +{
> > +   return plane_id - PLANE_SPRITE0;
> > +}
> > +
> >  static void vlv_invert_wms(struct intel_crtc *crtc)
> >  {
> >     struct vlv_wm_state *wm_state = &crtc->wm_state;
> > @@ -1079,7 +1085,7 @@ static void vlv_invert_wms(struct intel_crtc
> > *crtc)
> >                                     wm_state->wm[level].primary;
> >                             break;
> >                     case DRM_PLANE_TYPE_OVERLAY:
> > -                           sprite = plane->plane;
> > +                           sprite = vlv_sprite_id(plane->id);
> >                             wm_state->wm[level].sprite[sprite] =
> > plane->wm.fifo_size -
> >                                     wm_state-
> > >wm[level].sprite[sprite];
> >                             break;
> > @@ -1143,7 +1149,7 @@ static void vlv_compute_wm(struct intel_crtc
> > *crtc)
> >                             wm_state->wm[level].primary = wm;
> >                             break;
> >                     case DRM_PLANE_TYPE_OVERLAY:
> > -                           sprite = plane->plane;
> > +                           sprite = vlv_sprite_id(plane->id);
> >                             wm_state->wm[level].sprite[sprite] =
> > wm;
> >                             break;
> >                     }
> > @@ -1169,7 +1175,7 @@ static void vlv_compute_wm(struct intel_crtc
> > *crtc)
> >                                         wm_state-
> > >wm[level].primary);
> >                     break;
> >             case DRM_PLANE_TYPE_OVERLAY:
> > -                   sprite = plane->plane;
> > +                   sprite = vlv_sprite_id(plane->id);
> >                     for (level = 0; level < wm_state-
> > >num_levels; level++)
> >                             wm_state->sr[level].plane =
> >                                     min(wm_state-
> > >sr[level].plane,
> > @@ -1198,17 +1204,23 @@ static void vlv_pipe_set_fifo_size(struct
> > intel_crtc *crtc)
> >     int sprite0_start = 0, sprite1_start = 0, fifo_size = 0;
> >  
> >     for_each_intel_plane_on_crtc(dev, crtc, plane) {
> > -           if (plane->base.type == DRM_PLANE_TYPE_CURSOR) {
> > -                   WARN_ON(plane->wm.fifo_size != 63);
> > -                   continue;
> > -           }
> > -
> > -           if (plane->base.type == DRM_PLANE_TYPE_PRIMARY)
> > +           switch (plane->id) {
> > +           case PLANE_PRIMARY:
> >                     sprite0_start = plane->wm.fifo_size;
> > -           else if (plane->plane == 0)
> > +                   break;
> > +           case PLANE_SPRITE0:
> >                     sprite1_start = sprite0_start + plane-
> > >wm.fifo_size;
> > -           else
> > +                   break;
> > +           case PLANE_SPRITE1:
> >                     fifo_size = sprite1_start + plane-
> > >wm.fifo_size;
> > +                   break;
> > +           case PLANE_CURSOR:
> > +                   WARN_ON(plane->wm.fifo_size != 63);
> > +                   break;
> > +           default:
> > +                   MISSING_CASE(plane->id);
> > +                   break;
> > +           }
> >     }
> >  
> >     WARN_ON(fifo_size != 512 - 1);
> > @@ -4505,21 +4517,8 @@ void vlv_wm_get_hw_state(struct drm_device
> > *dev)
> >  
> >     vlv_read_wm_values(dev_priv, wm);
> >  
> > -   for_each_intel_plane(dev, plane) {
> > -           switch (plane->base.type) {
> > -                   int sprite;
> > -           case DRM_PLANE_TYPE_CURSOR:
> > -                   plane->wm.fifo_size = 63;
> > -                   break;
> > -           case DRM_PLANE_TYPE_PRIMARY:
> > -                   plane->wm.fifo_size =
> > vlv_get_fifo_size(dev_priv, plane->pipe, 0);
> > -                   break;
> > -           case DRM_PLANE_TYPE_OVERLAY:
> > -                   sprite = plane->plane;
> > -                   plane->wm.fifo_size =
> > vlv_get_fifo_size(dev_priv, plane->pipe, sprite + 1);
> > -                   break;
> > -           }
> > -   }
> > +   for_each_intel_plane(dev, plane)
> > +           plane->wm.fifo_size = vlv_get_fifo_size(plane);
> >  
> >     wm->cxsr = I915_READ(FW_BLC_SELF_VLV) & FW_CSPWRDWNEN;
> >     wm->level = VLV_WM_LEVEL_PM2;

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to