On Fri, Nov 18, 2016 at 12:25:30PM -0200, Paulo Zanoni wrote:
> Em Ter, 2016-11-08 às 16:47 +0200, ville.syrjala@xxxxxxxxxxxxxxx
> escreveu:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Now we've rename the local plane id variable as 'plane_id' everywhere
> > except the pre-SKL primary plane code. Let's do the rename there as
> > well
> > so that we'll free up the name 'plane' for use with struct
> > intel_plane*.
>
> As you pointed, my second email do patch 7 was supposed to be a reply
> to this patch. So let's move the discussion to the appropriate thread.
> And I'll try to better write my argumentation here.
>
> I think this series does a really nice job of consistently using the
> term "plane_id" to refer to the plane Z ordering that we have in the
> newer platforms, while keeping "plane" as the variable that refers to
> the HW plane (A/B/C) the struct represents. And, IMHO, this patch goes
> against that trend, since it starts using plane_id to talk about A/B/C.
>
> I see your goal is to leave the "plane" namespace restricted to
> variables that point to our plane structs, but I think that if we're
> going to do this, we should probably use something different than
> plane_id when talking about the hw a/b/c planes. So maybe using a
> different variable name here would solve the problem.
>
> What do you think?
Perhaps. But it's hard coming up with a name that pleases the eye
sufficiently.
>
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 57 +++++++++++++++++---------
> > ----------
> > 1 file changed, 27 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 95644c8cc568..bd084b085421 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3014,10 +3014,9 @@ static void i9xx_update_primary_plane(struct
> > drm_plane *primary,
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state-
> > >base.crtc);
> > struct drm_framebuffer *fb = plane_state->base.fb;
> > - int plane = intel_crtc->plane;
> > + enum plane plane_id = to_intel_plane(primary)->plane;
> > u32 linear_offset;
> > u32 dspcntr;
> > - i915_reg_t reg = DSPCNTR(plane);
> > unsigned int rotation = plane_state->base.rotation;
> > int x = plane_state->base.src.x1 >> 16;
> > int y = plane_state->base.src.y1 >> 16;
> > @@ -3033,16 +3032,16 @@ static void i9xx_update_primary_plane(struct
> > drm_plane *primary,
> > /* pipesrc and dspsize control the size that is
> > scaled from,
> > * which should always be the user's requested size.
> > */
> > - I915_WRITE(DSPSIZE(plane),
> > + I915_WRITE(DSPSIZE(plane_id),
> > ((crtc_state->pipe_src_h - 1) << 16) |
> > (crtc_state->pipe_src_w - 1));
> > - I915_WRITE(DSPPOS(plane), 0);
> > - } else if (IS_CHERRYVIEW(dev_priv) && plane == PLANE_B) {
> > - I915_WRITE(PRIMSIZE(plane),
> > + I915_WRITE(DSPPOS(plane_id), 0);
> > + } else if (IS_CHERRYVIEW(dev_priv) && plane_id == PLANE_B) {
> > + I915_WRITE(PRIMSIZE(plane_id),
> > ((crtc_state->pipe_src_h - 1) << 16) |
> > (crtc_state->pipe_src_w - 1));
> > - I915_WRITE(PRIMPOS(plane), 0);
> > - I915_WRITE(PRIMCNSTALPHA(plane), 0);
> > + I915_WRITE(PRIMPOS(plane_id), 0);
> > + I915_WRITE(PRIMCNSTALPHA(plane_id), 0);
> > }
> >
> > switch (fb->pixel_format) {
> > @@ -3099,21 +3098,21 @@ static void i9xx_update_primary_plane(struct
> > drm_plane *primary,
> > intel_crtc->adjusted_x = x;
> > intel_crtc->adjusted_y = y;
> >
> > - I915_WRITE(reg, dspcntr);
> > + I915_WRITE(DSPCNTR(plane_id), dspcntr);
> >
> > - I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
> > + I915_WRITE(DSPSTRIDE(plane_id), fb->pitches[0]);
> > if (INTEL_INFO(dev)->gen >= 4) {
> > - I915_WRITE(DSPSURF(plane),
> > + I915_WRITE(DSPSURF(plane_id),
> > intel_fb_gtt_offset(fb, rotation) +
> > intel_crtc->dspaddr_offset);
> > - I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
> > - I915_WRITE(DSPLINOFF(plane), linear_offset);
> > + I915_WRITE(DSPTILEOFF(plane_id), (y << 16) | x);
> > + I915_WRITE(DSPLINOFF(plane_id), linear_offset);
> > } else {
> > - I915_WRITE(DSPADDR(plane),
> > + I915_WRITE(DSPADDR(plane_id),
> > intel_fb_gtt_offset(fb, rotation) +
> > intel_crtc->dspaddr_offset);
> > }
> > - POSTING_READ(reg);
> > + POSTING_READ(DSPCNTR(plane_id));
> > }
> >
> > static void i9xx_disable_primary_plane(struct drm_plane *primary,
> > @@ -3121,15 +3120,14 @@ static void i9xx_disable_primary_plane(struct
> > drm_plane *primary,
> > {
> > struct drm_device *dev = crtc->dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > - int plane = intel_crtc->plane;
> > + enum plane plane_id = to_intel_plane(primary)->plane;
> >
> > - I915_WRITE(DSPCNTR(plane), 0);
> > + I915_WRITE(DSPCNTR(plane_id), 0);
> > if (INTEL_INFO(dev_priv)->gen >= 4)
> > - I915_WRITE(DSPSURF(plane), 0);
> > + I915_WRITE(DSPSURF(plane_id), 0);
> > else
> > - I915_WRITE(DSPADDR(plane), 0);
> > - POSTING_READ(DSPCNTR(plane));
> > + I915_WRITE(DSPADDR(plane_id), 0);
> > + POSTING_READ(DSPCNTR(plane_id));
> > }
> >
> > static void ironlake_update_primary_plane(struct drm_plane *primary,
> > @@ -3140,10 +3138,9 @@ static void
> > ironlake_update_primary_plane(struct drm_plane *primary,
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state-
> > >base.crtc);
> > struct drm_framebuffer *fb = plane_state->base.fb;
> > - int plane = intel_crtc->plane;
> > + enum plane plane_id = to_intel_plane(primary)->plane;
> > u32 linear_offset;
> > u32 dspcntr;
> > - i915_reg_t reg = DSPCNTR(plane);
> > unsigned int rotation = plane_state->base.rotation;
> > int x = plane_state->base.src.x1 >> 16;
> > int y = plane_state->base.src.y1 >> 16;
> > @@ -3202,19 +3199,19 @@ static void
> > ironlake_update_primary_plane(struct drm_plane *primary,
> > intel_crtc->adjusted_x = x;
> > intel_crtc->adjusted_y = y;
> >
> > - I915_WRITE(reg, dspcntr);
> > + I915_WRITE(DSPCNTR(plane_id), dspcntr);
> >
> > - I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
> > - I915_WRITE(DSPSURF(plane),
> > + I915_WRITE(DSPSTRIDE(plane_id), fb->pitches[0]);
> > + I915_WRITE(DSPSURF(plane_id),
> > intel_fb_gtt_offset(fb, rotation) +
> > intel_crtc->dspaddr_offset);
> > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > - I915_WRITE(DSPOFFSET(plane), (y << 16) | x);
> > + I915_WRITE(DSPOFFSET(plane_id), (y << 16) | x);
> > } else {
> > - I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
> > - I915_WRITE(DSPLINOFF(plane), linear_offset);
> > + I915_WRITE(DSPTILEOFF(plane_id), (y << 16) | x);
> > + I915_WRITE(DSPLINOFF(plane_id), linear_offset);
> > }
> > - POSTING_READ(reg);
> > + POSTING_READ(DSPCNTR(plane_id));
> > }
> >
> > u32 intel_fb_stride_alignment(const struct drm_i915_private
> > *dev_priv,
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
