On Fri, Nov 17, 2017 at 08:32:19PM +0000, Ville Syrjälä wrote:
> On Fri, Nov 17, 2017 at 12:12:15PM -0800, Rodrigo Vivi wrote:
> > On Fri, Nov 17, 2017 at 07:19:10PM +0000, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > >
> > > Rename enum plane to enum i9xx_plane_id to make it clear that it only
> > > applies to pre-SKL platforms.
> >
> > Oh! I should had read this before commenting on the cover letter... sorry...
> >
> > Anyway I don't believe it make clear that it is not skl... because it means
> > it started on i9xx, but not that ended on skl... just "plane" was already
> > good enough imho... but it is really just a comment...
> > don't take this as a nack anyhow... ;)
>
> I think our unwritten rule is that i9xx often refers to
> gen2/3/4, sometimes including g4x sometimes not, and sometimes
> even vlv/chv gets included.
>
> And sometimes we just call things by the name i915 instead of
> i9xx, meaning gmch. But on other occasions i915 means just gen3,
> and i9xx means anything else gmch not otherwise named more
> specifically.
>
> Got it? Good :)
hahaha I think so! :)
>
> In this case that "pattern" is a bit more broken since here
> i9xx extends all the way to bdw.
I was wondering something like that...
> But it does match the fact that
> the primary plane functions are now called i9xx_plane_whatever().
oh! I see...
>
> I think I need to include the cursor planes in the i9xx_plane_id
> soon enough, which will make it partially extend all the way to cnl
> and beyond. We really need to tell the hw people to redesign the
> cursor planes so that we have an excuse to split the code :P
in the hope that it wouldn't get it more fuzzy! :)
Thanks a lot for the explanation!
>
> >
> > >
> > > enum i9xx_plane_id is a global identifier, whereas enum plane_id is
> > > per-pipe. We need the old global identifier to index the primary plane
> > > (and the pre-g4x sprite C if we ever expose it) registers on pre-SKL
> > > platforms.
> > >
> > > v2: Reorder patches
> > > v3: s/old_plane_id/i9xx_plane_id/ (Daniel)
> > > Pimp the commit message a bit
> > > Note that i9xx_plane_id doesn't apply to SKL+
> > > v4: Rebase due to power domain handling in plane readout
> > > v5: Rebase due to crtc->dspaddr_offset removal
> > > v6: s/plane/i9xx_plane/ etc. (James)
> > >
> > > Cc: James Ausmus <james.ausmus@xxxxxxxxx>
> > > Cc: Daniel Vetter <daniel@xxxxxxxx>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h | 6 +--
> > > drivers/gpu/drm/i915/intel_display.c | 98 ++++++++++++++++++------------------
> > > drivers/gpu/drm/i915/intel_drv.h | 6 +--
> > > drivers/gpu/drm/i915/intel_fbc.c | 12 ++---
> > > drivers/gpu/drm/i915/intel_sprite.c | 2 +-
> > > 5 files changed, 62 insertions(+), 62 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 2158a758a17d..55dd602582cb 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -304,9 +304,9 @@ static inline bool transcoder_is_dsi(enum transcoder transcoder)
> > >
> > > /*
> > > * Global legacy plane identifier. Valid only for primary/sprite
> > > - * planes on pre-g4x, and only for primary planes on g4x+.
> > > + * planes on pre-g4x, and only for primary planes on g4x-bdw.
> > > */
> > > -enum plane {
> > > +enum i9xx_plane_id {
> > > PLANE_A,
> > > PLANE_B,
> > > PLANE_C,
> > > @@ -1145,7 +1145,7 @@ struct intel_fbc {
> > >
> > > struct {
> > > enum pipe pipe;
> > > - enum plane plane;
> > > + enum i9xx_plane_id i9xx_plane;
> > > unsigned int fence_y_offset;
> > > } crtc;
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 91f74c5373b3..16ac86816f28 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3220,16 +3220,16 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
> > > return 0;
> > > }
> > >
> > > -static void i9xx_update_primary_plane(struct intel_plane *primary,
> > > - const struct intel_crtc_state *crtc_state,
> > > - const struct intel_plane_state *plane_state)
> > > +static void i9xx_update_plane(struct intel_plane *plane,
> > > + const struct intel_crtc_state *crtc_state,
> > > + const struct intel_plane_state *plane_state)
> > > {
> > > - struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
> > > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > > const struct drm_framebuffer *fb = plane_state->base.fb;
> > > - enum plane plane = primary->plane;
> > > + enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
> > > u32 linear_offset;
> > > u32 dspcntr = plane_state->ctl;
> > > - i915_reg_t reg = DSPCNTR(plane);
> > > + i915_reg_t reg = DSPCNTR(i9xx_plane);
> > > int x = plane_state->main.x;
> > > int y = plane_state->main.y;
> > > unsigned long irqflags;
> > > @@ -3248,34 +3248,34 @@ static void i9xx_update_primary_plane(struct intel_plane *primary,
> > > /* pipesrc and dspsize control the size that is scaled from,
> > > * which should always be the user's requested size.
> > > */
> > > - I915_WRITE_FW(DSPSIZE(plane),
> > > + I915_WRITE_FW(DSPSIZE(i9xx_plane),
> > > ((crtc_state->pipe_src_h - 1) << 16) |
> > > (crtc_state->pipe_src_w - 1));
> > > - I915_WRITE_FW(DSPPOS(plane), 0);
> > > - } else if (IS_CHERRYVIEW(dev_priv) && plane == PLANE_B) {
> > > - I915_WRITE_FW(PRIMSIZE(plane),
> > > + I915_WRITE_FW(DSPPOS(i9xx_plane), 0);
> > > + } else if (IS_CHERRYVIEW(dev_priv) && i9xx_plane == PLANE_B) {
> > > + I915_WRITE_FW(PRIMSIZE(i9xx_plane),
> > > ((crtc_state->pipe_src_h - 1) << 16) |
> > > (crtc_state->pipe_src_w - 1));
> > > - I915_WRITE_FW(PRIMPOS(plane), 0);
> > > - I915_WRITE_FW(PRIMCNSTALPHA(plane), 0);
> > > + I915_WRITE_FW(PRIMPOS(i9xx_plane), 0);
> > > + I915_WRITE_FW(PRIMCNSTALPHA(i9xx_plane), 0);
> > > }
> > >
> > > I915_WRITE_FW(reg, dspcntr);
> > >
> > > - I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
> > > + I915_WRITE_FW(DSPSTRIDE(i9xx_plane), fb->pitches[0]);
> > > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > - I915_WRITE_FW(DSPSURF(plane),
> > > + I915_WRITE_FW(DSPSURF(i9xx_plane),
> > > intel_plane_ggtt_offset(plane_state) +
> > > dspaddr_offset);
> > > - I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
> > > + I915_WRITE_FW(DSPOFFSET(i9xx_plane), (y << 16) | x);
> > > } else if (INTEL_GEN(dev_priv) >= 4) {
> > > - I915_WRITE_FW(DSPSURF(plane),
> > > + I915_WRITE_FW(DSPSURF(i9xx_plane),
> > > intel_plane_ggtt_offset(plane_state) +
> > > dspaddr_offset);
> > > - I915_WRITE_FW(DSPTILEOFF(plane), (y << 16) | x);
> > > - I915_WRITE_FW(DSPLINOFF(plane), linear_offset);
> > > + I915_WRITE_FW(DSPTILEOFF(i9xx_plane), (y << 16) | x);
> > > + I915_WRITE_FW(DSPLINOFF(i9xx_plane), linear_offset);
> > > } else {
> > > - I915_WRITE_FW(DSPADDR(plane),
> > > + I915_WRITE_FW(DSPADDR(i9xx_plane),
> > > intel_plane_ggtt_offset(plane_state) +
> > > dspaddr_offset);
> > > }
> > > @@ -3284,32 +3284,31 @@ static void i9xx_update_primary_plane(struct intel_plane *primary,
> > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> > > }
> > >
> > > -static void i9xx_disable_primary_plane(struct intel_plane *primary,
> > > - struct intel_crtc *crtc)
> > > +static void i9xx_disable_plane(struct intel_plane *plane,
> > > + struct intel_crtc *crtc)
> > > {
> > > - struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
> > > - enum plane plane = primary->plane;
> > > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > > + enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
> > > unsigned long irqflags;
> > >
> > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> > >
> > > - I915_WRITE_FW(DSPCNTR(plane), 0);
> > > - if (INTEL_INFO(dev_priv)->gen >= 4)
> > > - I915_WRITE_FW(DSPSURF(plane), 0);
> > > + I915_WRITE_FW(DSPCNTR(i9xx_plane), 0);
> > > + if (INTEL_GEN(dev_priv) >= 4)
> > > + I915_WRITE_FW(DSPSURF(i9xx_plane), 0);
> > > else
> > > - I915_WRITE_FW(DSPADDR(plane), 0);
> > > - POSTING_READ_FW(DSPCNTR(plane));
> > > + I915_WRITE_FW(DSPADDR(i9xx_plane), 0);
> > > + POSTING_READ_FW(DSPCNTR(i9xx_plane));
> > >
> > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> > > }
> > >
> > > -static bool i9xx_plane_get_hw_state(struct intel_plane *primary)
> > > +static bool i9xx_plane_get_hw_state(struct intel_plane *plane)
> > > {
> > > -
> > > - struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
> > > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > > enum intel_display_power_domain power_domain;
> > > - enum plane plane = primary->plane;
> > > - enum pipe pipe = primary->pipe;
> > > + enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
> > > + enum pipe pipe = plane->pipe;
> > > bool ret;
> > >
> > > /*
> > > @@ -3321,7 +3320,7 @@ static bool i9xx_plane_get_hw_state(struct intel_plane *primary)
> > > if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> > > return false;
> > >
> > > - ret = I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE;
> > > + ret = I915_READ(DSPCNTR(i9xx_plane)) & DISPLAY_PLANE_ENABLE;
> > >
> > > intel_display_power_put(dev_priv, power_domain);
> > >
> > > @@ -7406,7 +7405,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
> > > struct drm_device *dev = crtc->base.dev;
> > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > u32 val, base, offset;
> > > - int pipe = crtc->pipe, plane = crtc->plane;
> > > + int pipe = crtc->pipe, plane = crtc->i9xx_plane;
> > > int fourcc, pixel_format;
> > > unsigned int aligned_height;
> > > struct drm_framebuffer *fb;
> > > @@ -13272,9 +13271,9 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> > > * port is hooked to pipe B. Hence we want plane A feeding pipe B.
> > > */
> > > if (HAS_FBC(dev_priv) && INTEL_GEN(dev_priv) < 4)
> > > - primary->plane = (enum plane) !pipe;
> > > + primary->i9xx_plane = (enum i9xx_plane_id) !pipe;
> > > else
> > > - primary->plane = (enum plane) pipe;
> > > + primary->i9xx_plane = (enum i9xx_plane_id) pipe;
> > > primary->id = PLANE_PRIMARY;
> > > primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
> > > primary->check_plane = intel_check_primary_plane;
> > > @@ -13303,16 +13302,16 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> > > num_formats = ARRAY_SIZE(i965_primary_formats);
> > > modifiers = i9xx_format_modifiers;
> > >
> > > - primary->update_plane = i9xx_update_primary_plane;
> > > - primary->disable_plane = i9xx_disable_primary_plane;
> > > + primary->update_plane = i9xx_update_plane;
> > > + primary->disable_plane = i9xx_disable_plane;
> > > primary->get_hw_state = i9xx_plane_get_hw_state;
> > > } else {
> > > intel_primary_formats = i8xx_primary_formats;
> > > num_formats = ARRAY_SIZE(i8xx_primary_formats);
> > > modifiers = i9xx_format_modifiers;
> > >
> > > - primary->update_plane = i9xx_update_primary_plane;
> > > - primary->disable_plane = i9xx_disable_primary_plane;
> > > + primary->update_plane = i9xx_update_plane;
> > > + primary->disable_plane = i9xx_disable_plane;
> > > primary->get_hw_state = i9xx_plane_get_hw_state;
> > > }
> > >
> > > @@ -13336,7 +13335,8 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> > > intel_primary_formats, num_formats,
> > > modifiers,
> > > DRM_PLANE_TYPE_PRIMARY,
> > > - "plane %c", plane_name(primary->plane));
> > > + "plane %c",
> > > + plane_name(primary->i9xx_plane));
> > > if (ret)
> > > goto fail;
> > >
> > > @@ -13396,7 +13396,7 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
> > > cursor->can_scale = false;
> > > cursor->max_downscale = 1;
> > > cursor->pipe = pipe;
> > > - cursor->plane = pipe;
> > > + cursor->i9xx_plane = (enum i9xx_plane_id) pipe;
> > > cursor->id = PLANE_CURSOR;
> > > cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
> > >
> > > @@ -13524,14 +13524,14 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
> > > goto fail;
> > >
> > > intel_crtc->pipe = pipe;
> > > - intel_crtc->plane = primary->plane;
> > > + intel_crtc->i9xx_plane = primary->i9xx_plane;
> > >
> > > /* initialize shared scalers */
> > > intel_crtc_init_scalers(intel_crtc, crtc_state);
> > >
> > > BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
> > > - dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
> > > - dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = intel_crtc;
> > > + dev_priv->plane_to_crtc_mapping[intel_crtc->i9xx_plane] != NULL);
> > > + dev_priv->plane_to_crtc_mapping[intel_crtc->i9xx_plane] = intel_crtc;
> > > dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = intel_crtc;
> > >
> > > drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
> > > @@ -14788,11 +14788,11 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
> > > }
> > >
> > > static bool intel_plane_mapping_ok(struct intel_crtc *crtc,
> > > - struct intel_plane *primary)
> > > + struct intel_plane *plane)
> > > {
> > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > - enum plane plane = primary->plane;
> > > - u32 val = I915_READ(DSPCNTR(plane));
> > > + enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
> > > + u32 val = I915_READ(DSPCNTR(i9xx_plane));
> > >
> > > return (val & DISPLAY_PLANE_ENABLE) == 0 ||
> > > (val & DISPPLANE_SEL_PIPE_MASK) == DISPPLANE_SEL_PIPE(crtc->pipe);
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 99840f3940c7..d1fe7be94b62 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -799,7 +799,7 @@ struct intel_crtc_state {
> > > struct intel_crtc {
> > > struct drm_crtc base;
> > > enum pipe pipe;
> > > - enum plane plane;
> > > + enum i9xx_plane_id i9xx_plane;
> > > /*
> > > * Whether the crtc and the connected output pipeline is active. Implies
> > > * that crtc->enabled is set, i.e. the current mode configuration has
> > > @@ -844,7 +844,7 @@ struct intel_crtc {
> > >
> > > struct intel_plane {
> > > struct drm_plane base;
> > > - u8 plane;
> > > + enum i9xx_plane_id i9xx_plane;
> > > enum plane_id id;
> > > enum pipe pipe;
> > > bool can_scale;
> > > @@ -1130,7 +1130,7 @@ intel_get_crtc_for_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
> > > }
> > >
> > > static inline struct intel_crtc *
> > > -intel_get_crtc_for_plane(struct drm_i915_private *dev_priv, enum plane plane)
> > > +intel_get_crtc_for_plane(struct drm_i915_private *dev_priv, enum i9xx_plane_id plane)
> > > {
> > > return dev_priv->plane_to_crtc_mapping[plane];
> > > }
> > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > > index 1a0f5e0c8d10..3133131306a9 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > > @@ -151,7 +151,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
> > >
> > > /* Set it up... */
> > > fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
> > > - fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.plane);
> > > + fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.i9xx_plane);
> > > I915_WRITE(FBC_CONTROL2, fbc_ctl2);
> > > I915_WRITE(FBC_FENCE_OFF, params->crtc.fence_y_offset);
> > > }
> > > @@ -177,7 +177,7 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
> > > struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
> > > u32 dpfc_ctl;
> > >
> > > - dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane) | DPFC_SR_EN;
> > > + dpfc_ctl = DPFC_CTL_PLANE(params->crtc.i9xx_plane) | DPFC_SR_EN;
> > > if (params->fb.format->cpp[0] == 2)
> > > dpfc_ctl |= DPFC_CTL_LIMIT_2X;
> > > else
> > > @@ -224,7 +224,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
> > > u32 dpfc_ctl;
> > > int threshold = dev_priv->fbc.threshold;
> > >
> > > - dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane);
> > > + dpfc_ctl = DPFC_CTL_PLANE(params->crtc.i9xx_plane);
> > > if (params->fb.format->cpp[0] == 2)
> > > threshold++;
> > >
> > > @@ -306,7 +306,7 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
> > >
> > > dpfc_ctl = 0;
> > > if (IS_IVYBRIDGE(dev_priv))
> > > - dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.plane);
> > > + dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.i9xx_plane);
> > >
> > > if (params->fb.format->cpp[0] == 2)
> > > threshold++;
> > > @@ -890,7 +890,7 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
> > > params->vma = cache->vma;
> > >
> > > params->crtc.pipe = crtc->pipe;
> > > - params->crtc.plane = crtc->plane;
> > > + params->crtc.i9xx_plane = crtc->i9xx_plane;
> > > params->crtc.fence_y_offset = get_crtc_fence_y_offset(fbc);
> > >
> > > params->fb.format = cache->fb.format;
> > > @@ -1088,7 +1088,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
> > > if (fbc_on_pipe_a_only(dev_priv) && crtc->pipe != PIPE_A)
> > > continue;
> > >
> > > - if (fbc_on_plane_a_only(dev_priv) && crtc->plane != PLANE_A)
> > > + if (fbc_on_plane_a_only(dev_priv) && crtc->i9xx_plane != PLANE_A)
> > > continue;
> > >
> > > intel_crtc_state = to_intel_crtc_state(
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > index 5baa0023964e..dd485f59eb1d 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -1384,7 +1384,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
> > > }
> > >
> > > intel_plane->pipe = pipe;
> > > - intel_plane->plane = plane;
> > > + intel_plane->i9xx_plane = plane;
> > > intel_plane->id = PLANE_SPRITE0 + plane;
> > > intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
> > > intel_plane->check_plane = intel_check_sprite_plane;
> > > --
> > > 2.13.6
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
