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?
>
> 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,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
