[Intel-gfx] [RFC 3/4] drm/i915/gen9: Expose top-most universal plane as cursor
Lyude Paul
lyude at redhat.com
Thu Oct 27 22:55:57 UTC 2016
On Thu, 2016-10-27 at 15:35 -0700, Matt Roper wrote:
> On Thu, Oct 27, 2016 at 06:15:32PM -0400, Lyude Paul wrote:
> >
> > On Wed, 2016-10-26 at 15:51 -0700, Matt Roper wrote:
> > >
> > > Gen9 has a traditional cursor plane that is mutually exclusive
> > > with
> > > the
> > > system's top-most "universal" plane; it seems likely that two
> > > planes
> > > are
> > > really a single shared hardware unit with two different register
> > > interfaces. Thus far i915 has exposed a cursor plane to
> > > userspace
> > > that's hooked up to the old-style cursor registers; we just
> > > pretended
> > > that the top-most universal plane didn't exist and reported one
> > > fewer
> > > "sprite/overlay" planes for each pipe than the platform
> > > technically
> > > has.
> > > Let's switch this around so that the cursor exposed to userspace
> > > is
> > > actually wired up to top-most universal plane registers. We'll
> > > continue
> > > to present the same cursor ABI to userspace that we always have,
> > > but
> > > internally we'll just be programming a different set of registers
> > > and
> > > doing so in a way that's more consistent with how all the other
> > > gen9
> > > planes work --- less cursor-specific special cases throughout the
> > > code.
> > >
> > > Aside from making the code a bit simpler (fewer cursor-specific
> > > special
> > > cases), this will also pave the way to eventually exposing the
> > > top-
> > > most
> > > plane in a more full-featured manner to userspace clients that
> > > don't
> > > need a traditional cursor and wish to opt into having access to
> > > an
> > > additional sprite/overlay-style plane instead.
> > >
> > > It's worth noting that a lot of the special-cased cursor-specific
> > > code
> > > was in the gen9 watermark programming. It's good to simplify
> > > that
> > > code,
> > > but we should keep an eye out for any unwanted side effects of
> > > this
> > > patch; since sprites/overlays tend to get used less than cursors,
> > > it's
> > > possible that this could help us uncover additional underruns
> > > that
> > > nobody had run across yet. Or it could have the opposite effect
> > > and
> > > eliminate some of the underruns that we haven't been able to get
> > > rid
> > > of
> > > yet.
> >
> > Alright, so I had to sit on this patch for a while. I think
> > exposing
> > this as a normal plane is a good idea: the rest of the world just
> > uses
> > planes, so we should too. We need to be *really* careful with this
> > though. Like Paulo said watermarks are still fragile and honestly I
> > wouldn't be surprised if we found more underruns hiding somewhere.
>
> Agreed. We're programming a different subset of the internal
> hardware
> so even if we do it 100% properly according to the spec, we can
> easily
> uncover previously hidden painpoints in the hardware that need new
> workarounds.
>
> >
> > First off, although this was mentioned in an e-mail down the chain,
> > I'm
> > pretty sure having a 0 block allocation is definitely not what we
> > want.
> > The spec itself says a minimum of 8 blocks assuming it's a normal
> > cursor, so if we're going below that we definitely need to at least
> > make sure that behavior isn't wrong.
>
> DDB allocation should only be necessary if a plane is being used
> (otherwise you have no data to fill the buffer with anyway). If a
> plane
Ah whoops, I thought you were implying that we had 0 block ddb
allocation on active planes.
> is off, then it shouldn't need allocation (and this patch guarantees
> that the cursor will _never_ be turned on) I'm pretty sure we've
> actually had some threads where Art (one of our hardware architects)
> mentioned that the cursor fixed allocation is just an optimization
> and
> isn't in any way required even when you do use the cursor hardware
> plane. The only one I can find searching now is this one:
>
> https://lists.freedesktop.org/archives/intel-gfx/2016-June/09
> 9256.html
>
> but I feel like there was another thread (or maybe an IRC
> conversation)
> where he talked about it a bit more. I'll see if I can find the one
> I'm
> thinking of.
>
> >
> > The second thing is that unfortunately this regresses in a rather
> > strange way. On this X1 yoga w/ a 4K screen (SKL chipset of
> > course), if
> > I move the cursor in front of a gray background I can see a faintly
> > visible box starting from the beginning of the cursor and going to
> > the
> > end of the plane. Reverting the patch fixes the problem.
>
> Yeah, I mentioned that in the cover letter...I believe the color
> correction settings are different for the universal plane vs the
> cursor
> plane (which causes IGT CRC mismatches at the moment and may be
> visually
> noticeable if you have good eyes); that shouldn't be hard to track
Ah whoops, must have missed that part. Regardless though, we should
probably get those out of the way first before applying any patches
like this. Otherwise I think the patch looks good, so long as we can
get a couple of people to try it out on their systems for a while and
see if anything breaks.
> down
> and fix.
>
>
> Matt
>
> >
> >
> > >
> > >
> > > Cc: Bob Paauwe <bob.j.paauwe at intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > > Cc: Lyude Paul <lyude at redhat.com>
> > > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_debugfs.c | 4 --
> > > drivers/gpu/drm/i915/i915_drv.h | 11 +++-
> > > drivers/gpu/drm/i915/intel_device_info.c | 38 +++++++++----
> > > drivers/gpu/drm/i915/intel_display.c | 97 ++++++++++++----
> > > ----
> > > ------------
> > > drivers/gpu/drm/i915/intel_drv.h | 7 +++
> > > drivers/gpu/drm/i915/intel_pm.c | 85 ++++------------
> > > ----
> > > --------
> > > drivers/gpu/drm/i915/intel_sprite.c | 6 +-
> > > 7 files changed, 93 insertions(+), 155 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 9f5a392..0bba472 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -3469,10 +3469,6 @@ static int i915_ddb_info(struct seq_file
> > > *m,
> > > void *unused)
> > > entry->start, entry->end,
> > > skl_ddb_entry_size(entry));
> > > }
> > > -
> > > - entry = &ddb->plane[pipe][PLANE_CURSOR];
> > > - seq_printf(m, " %-13s%8u%8u%8u\n", "Cursor",
> > > entry-
> > > >
> > > > start,
> > > - entry->end,
> > > skl_ddb_entry_size(entry));
> > > }
> > >
> > > drm_modeset_unlock_all(dev);
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 4714051..83aaed2 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -178,7 +178,7 @@ enum plane {
> > > PLANE_A = 0,
> > > PLANE_B,
> > > PLANE_C,
> > > - PLANE_CURSOR,
> > > + PLANE_D,
> > > I915_MAX_PLANES,
> > > };
> > > #define plane_name(p) ((p) + 'A')
> > > @@ -316,9 +316,15 @@ struct i915_hotplug {
> > > for ((__p) = 0;
> > > \
> > > (__p) < INTEL_INFO(__dev_priv)-
> > > >num_sprites[(__pipe)] +
> > > 1; \
> > > (__p)++)
> > > +
> > > +/*
> > > + * Only iterate over sprites exposed as sprites; omit sprites
> > > that
> > > + * are being repurposed to simulate a cursor.
> > > + */
> > > #define for_each_sprite(__dev_priv, __p, __s)
> > > \
> > > for ((__s) = 0;
> > > \
> > > - (__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)];
> > >
> > > \
> > > + (__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)]
> > > -
> > > \
> > > + (INTEL_INFO(__dev_priv)->has_real_cursor ?
> > > 0 :
> > > 1); \
> > > (__s)++)
> > >
> > > #define for_each_port_masked(__port, __ports_mask) \
> > > @@ -687,6 +693,7 @@ struct intel_csr {
> > > func(has_psr); \
> > > func(has_rc6); \
> > > func(has_rc6p); \
> > > + func(has_real_cursor); \
> > > func(has_resource_streamer); \
> > > func(has_runtime_pm); \
> > > func(has_snoop); \
> > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> > > b/drivers/gpu/drm/i915/intel_device_info.c
> > > index d6a8f11..a464e0e 100644
> > > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > > @@ -271,23 +271,39 @@ void intel_device_info_runtime_init(struct
> > > drm_i915_private *dev_priv)
> > > enum pipe pipe;
> > >
> > > /*
> > > - * Skylake and Broxton currently don't expose the
> > > topmost
> > > plane as its
> > > - * use is exclusive with the legacy cursor and we only
> > > want
> > > to expose
> > > - * one of those, not both. Until we can safely expose
> > > the
> > > topmost plane
> > > - * as a DRM_PLANE_TYPE_CURSOR with all the features
> > > exposed/supported,
> > > - * we don't expose the topmost plane at all to prevent
> > > ABI
> > > breakage
> > > - * down the line.
> > > + * Gen9 platforms have a top-most universal (i.e.,
> > > sprite)
> > > plane and a
> > > + * cursor plane that are mutually exclusive. If we use
> > > the
> > > cursor
> > > + * plane we permanently lose the ability to make use of
> > > the
> > > more
> > > + * full-featured universal plane. So instead let's use
> > > all
> > > of the
> > > + * universal planes, ignore the cursor plane, but hook
> > > the
> > > top-most
> > > + * universal plane up to the legacy cursor ioctl's and
> > > expose it to
> > > + * userspace as DRM_PLANE_TYPE_CURSOR. This won't
> > > result in
> > > any
> > > + * visible behavior change to userspace; we're just
> > > internally
> > > + * programming a different set of registers.
> > > + *
> > > + * For the purposes of device_info, we're only concerned
> > > with the
> > > + * number of universal planes we're programming,
> > > regardless
> > > of how they
> > > + * get mapped to userspace interfaces, so we'll report
> > > the
> > > true number of
> > > + * universal planes for gen9.
> > > */
> > > if (IS_BROXTON(dev_priv)) {
> > > - info->num_sprites[PIPE_A] = 2;
> > > - info->num_sprites[PIPE_B] = 2;
> > > - info->num_sprites[PIPE_C] = 1;
> > > - } else if (IS_VALLEYVIEW(dev_priv) ||
> > > IS_CHERRYVIEW(dev_priv))
> > > + info->has_real_cursor = 0;
> > > + info->num_sprites[PIPE_A] = 3;
> > > + info->num_sprites[PIPE_B] = 3;
> > > + info->num_sprites[PIPE_C] = 2;
> > > + } else if (IS_GEN9(dev_priv)) {
> > > + info->has_real_cursor = 0;
> > > for_each_pipe(dev_priv, pipe)
> > > info->num_sprites[pipe] = 2;
> > > - else
> > > + } else if (IS_VALLEYVIEW(dev_priv) ||
> > > IS_CHERRYVIEW(dev_priv)) {
> > > + info->has_real_cursor = 1;
> > > + for_each_pipe(dev_priv, pipe)
> > > + info->num_sprites[pipe] = 2;
> > > + } else {
> > > + info->has_real_cursor = 1;
> > > for_each_pipe(dev_priv, pipe)
> > > info->num_sprites[pipe] = 1;
> > > + }
> > >
> > > if (i915.disable_display) {
> > > DRM_INFO("Display disabled (module
> > > parameter)\n");
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index cb7dd11..9a8c2b1 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -1232,6 +1232,23 @@ void assert_panel_unlocked(struct
> > > drm_i915_private *dev_priv, enum pipe pipe)
> > > pipe_name(pipe));
> > > }
> > >
> > > +/*
> > > + * Cursor state for platforms that use a universal plane as a
> > > cursor.
> > > + * Primary is universal #0, others are universal #1-
> > > numsprites. Cursor
> > > + * will be the final universal plane for the pipe.
> > > + */
> > > +static bool
> > > +universal_cursor_state(struct drm_i915_private *dev_priv,
> > > + enum pipe pipe)
> > > +{
> > > + unsigned int planenum = INTEL_INFO(dev_priv)-
> > > >
> > > > num_sprites[pipe];
> > > + u32 val;
> > > +
> > > + val = I915_READ(PLANE_CTL(pipe, planenum));
> > > +
> > > + return val & PLANE_CTL_ENABLE;
> > > +}
> > > +
> > > static void assert_cursor(struct drm_i915_private *dev_priv,
> > > enum pipe pipe, bool state)
> > > {
> > > @@ -1239,6 +1256,8 @@ static void assert_cursor(struct
> > > drm_i915_private *dev_priv,
> > >
> > > if (IS_845G(dev_priv) || IS_I865G(dev_priv))
> > > cur_state = I915_READ(CURCNTR(PIPE_A)) &
> > > CURSOR_ENABLE;
> > > + else if (!INTEL_INFO(dev_priv)->has_real_cursor)
> > > + cur_state = universal_cursor_state(dev_priv,
> > > pipe);
> > > else
> > > cur_state = I915_READ(CURCNTR(pipe)) &
> > > CURSOR_MODE;
> > >
> > > @@ -10841,15 +10860,16 @@ static void i9xx_update_cursor(struct
> > > drm_crtc *crtc, u32 base,
> > > struct drm_device *dev = crtc->dev;
> > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > > - struct intel_crtc_state *cstate =
> > > to_intel_crtc_state(crtc-
> > > >
> > > > state);
> > > - const struct skl_wm_values *wm = &dev_priv-
> > > >wm.skl_results;
> > > - const struct skl_plane_wm *p_wm =
> > > - &cstate->wm.skl.optimal.planes[PLANE_CURSOR];
> > > int pipe = intel_crtc->pipe;
> > > uint32_t cntl = 0;
> > >
> > > - if (INTEL_GEN(dev_priv) >= 9 && wm->dirty_pipes &
> > > drm_crtc_mask(crtc))
> > > - skl_write_cursor_wm(intel_crtc, p_wm, &wm->ddb);
> > > + /*
> > > + * Although gen9 has legacy cursor registers, their use
> > > is
> > > mutually
> > > + * exclusive with the top-most universal plane. We'll
> > > just
> > > use the
> > > + * universal plane to simulate the legacy cursor
> > > interface
> > > instead,
> > > + * which means we'll never enter here on gen9 platforms.
> > > + */
> > > + WARN_ON(IS_GEN9(dev_priv));
> > >
> > > if (plane_state && plane_state->base.visible) {
> > > cntl = MCURSOR_GAMMA_ENABLE;
> > > @@ -13528,56 +13548,6 @@ static void verify_wm_state(struct
> > > drm_crtc
> > > *crtc,
> > > hw_ddb_entry->start,
> > > hw_ddb_entry-
> > > >
> > > > end);
> > > }
> > > }
> > > -
> > > - /*
> > > - * cursor
> > > - * If the cursor plane isn't active, we may not have
> > > updated
> > > it's ddb
> > > - * allocation. In that case since the ddb allocation
> > > will be
> > > updated
> > > - * once the plane becomes visible, we can skip this
> > > check
> > > - */
> > > - if (intel_crtc->cursor_addr) {
> > > - hw_plane_wm = &hw_wm.planes[PLANE_CURSOR];
> > > - sw_plane_wm = &sw_wm->planes[PLANE_CURSOR];
> > > -
> > > - /* Watermarks */
> > > - for (level = 0; level <= max_level; level++) {
> > > - if (skl_wm_level_equals(&hw_plane_wm-
> > > >
> > > > wm[level],
> > > - &sw_plane_wm-
> > > >
> > > > wm[level]))
> > > - continue;
> > > -
> > > - DRM_ERROR("mismatch in WM pipe %c cursor
> > > level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> > > - pipe_name(pipe), level,
> > > - sw_plane_wm-
> > > >wm[level].plane_en,
> > > - sw_plane_wm-
> > > >
> > > > wm[level].plane_res_b,
> > > - sw_plane_wm-
> > > >
> > > > wm[level].plane_res_l,
> > > - hw_plane_wm-
> > > >wm[level].plane_en,
> > > - hw_plane_wm-
> > > >
> > > > wm[level].plane_res_b,
> > > - hw_plane_wm-
> > > >
> > > > wm[level].plane_res_l);
> > > - }
> > > -
> > > - if (!skl_wm_level_equals(&hw_plane_wm->trans_wm,
> > > - &sw_plane_wm-
> > > >trans_wm)) {
> > > - DRM_ERROR("mismatch in trans WM pipe %c
> > > cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> > > - pipe_name(pipe),
> > > - sw_plane_wm-
> > > >trans_wm.plane_en,
> > > - sw_plane_wm-
> > > >trans_wm.plane_res_b,
> > > - sw_plane_wm-
> > > >trans_wm.plane_res_l,
> > > - hw_plane_wm-
> > > >trans_wm.plane_en,
> > > - hw_plane_wm-
> > > >trans_wm.plane_res_b,
> > > - hw_plane_wm-
> > > >
> > > > trans_wm.plane_res_l);
> > > - }
> > > -
> > > - /* DDB */
> > > - hw_ddb_entry =
> > > &hw_ddb.plane[pipe][PLANE_CURSOR];
> > > - sw_ddb_entry = &sw_ddb-
> > > >plane[pipe][PLANE_CURSOR];
> > > -
> > > - if (!skl_ddb_entry_equal(hw_ddb_entry,
> > > sw_ddb_entry)) {
> > > - DRM_ERROR("mismatch in DDB state pipe %c
> > > cursor (expected (%u,%u), found (%u,%u))\n",
> > > - pipe_name(pipe),
> > > - sw_ddb_entry->start,
> > > sw_ddb_entry-
> > > >
> > > > end,
> > > - hw_ddb_entry->start,
> > > hw_ddb_entry-
> > > >
> > > > end);
> > > - }
> > > - }
> > > }
> > >
> > > static void
> > > @@ -15215,11 +15185,18 @@ static struct drm_plane
> > > *intel_cursor_plane_create(struct drm_device *dev,
> > > cursor->can_scale = false;
> > > cursor->max_downscale = 1;
> > > cursor->pipe = pipe;
> > > - cursor->plane = pipe;
> > > cursor->frontbuffer_bit =
> > > INTEL_FRONTBUFFER_CURSOR(pipe);
> > > - cursor->check_plane = intel_check_cursor_plane;
> > > - cursor->update_plane = intel_update_cursor_plane;
> > > - cursor->disable_plane = intel_disable_cursor_plane;
> > > + if (INTEL_INFO(dev)->has_real_cursor) {
> > > + cursor->plane = pipe; /* unused? */
> > > + cursor->check_plane = intel_check_cursor_plane;
> > > + cursor->update_plane =
> > > intel_update_cursor_plane;
> > > + cursor->disable_plane =
> > > intel_disable_cursor_plane;
> > > + } else {
> > > + cursor->plane = INTEL_INFO(dev)-
> > > >num_sprites[pipe] -
> > > 1;
> > > + cursor->check_plane = intel_check_sprite_plane;
> > > + cursor->update_plane = skl_update_plane;
> > > + cursor->disable_plane = skl_disable_plane;
> > > + }
> > >
> > > ret = drm_universal_plane_init(dev, &cursor->base, 0,
> > > &intel_plane_funcs,
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index c31fddd..50874e2 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1790,6 +1790,13 @@ int intel_sprite_set_colorkey(struct
> > > drm_device *dev, void *data,
> > > struct drm_file *file_priv);
> > > void intel_pipe_update_start(struct intel_crtc *crtc);
> > > void intel_pipe_update_end(struct intel_crtc *crtc, struct
> > > intel_flip_work *work);
> > > +int intel_check_sprite_plane(struct drm_plane *plane,
> > > + struct intel_crtc_state
> > > *crtc_state,
> > > + struct intel_plane_state *state);
> > > +void skl_update_plane(struct drm_plane *drm_plane,
> > > + const struct intel_crtc_state *crtc_state,
> > > + const struct intel_plane_state
> > > *plane_state);
> > > +void skl_disable_plane(struct drm_plane *dplane, struct drm_crtc
> > > *crtc);
> > >
> > > /* intel_tv.c */
> > > void intel_tv_init(struct drm_device *dev);
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > b/drivers/gpu/drm/i915/intel_pm.c
> > > index 6f19e60..e75f6d8 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -2863,9 +2863,7 @@ bool ilk_disable_lp_wm(struct drm_device
> > > *dev)
> > >
> > > /*
> > > * Return the index of a plane in the SKL DDB and wm result
> > > arrays. Primary
> > > - * plane is always in slot 0, cursor is always in slot
> > > I915_MAX_PLANES-1, and
> > > - * other universal planes are in indices 1..n. Note that this
> > > may
> > > leave unused
> > > - * indices between the top "sprite" plane and the cursor.
> > > + * plane is always in slot 0 and other universal planes are in
> > > indices 1..n.
> > > */
> > > static int
> > > skl_wm_plane_id(const struct intel_plane *plane)
> > > @@ -2874,7 +2872,6 @@ skl_wm_plane_id(const struct intel_plane
> > > *plane)
> > > case DRM_PLANE_TYPE_PRIMARY:
> > > return 0;
> > > case DRM_PLANE_TYPE_CURSOR:
> > > - return PLANE_CURSOR;
> > > case DRM_PLANE_TYPE_OVERLAY:
> > > return plane->plane + 1;
> > > default:
> > > @@ -3128,14 +3125,6 @@ skl_ddb_get_pipe_allocation_limits(struct
> > > drm_device *dev,
> > > alloc->end = alloc->start + pipe_size;
> > > }
> > >
> > > -static unsigned int skl_cursor_allocation(int num_active)
> > > -{
> > > - if (num_active == 1)
> > > - return 32;
> > > -
> > > - return 8;
> > > -}
> > > -
> > > static void skl_ddb_entry_init_from_hw(struct skl_ddb_entry
> > > *entry,
> > > u32 reg)
> > > {
> > > entry->start = reg & 0x3ff;
> > > @@ -3166,10 +3155,6 @@ void skl_ddb_get_hw_state(struct
> > > drm_i915_private *dev_priv,
> > > val);
> > > }
> > >
> > > - val = I915_READ(CUR_BUF_CFG(pipe));
> > > - skl_ddb_entry_init_from_hw(&ddb-
> > > >
> > > > plane[pipe][PLANE_CURSOR],
> > > - val);
> > > -
> > > intel_display_power_put(dev_priv, power_domain);
> > > }
> > > }
> > > @@ -3227,8 +3212,6 @@ skl_plane_relative_data_rate(const struct
> > > intel_crtc_state *cstate,
> > >
> > > if (!intel_pstate->base.visible)
> > > return 0;
> > > - if (pstate->plane->type == DRM_PLANE_TYPE_CURSOR)
> > > - return 0;
> > > if (y && format != DRM_FORMAT_NV12)
> > > return 0;
> > >
> > > @@ -3386,7 +3369,7 @@ skl_allocate_pipe_ddb(struct
> > > intel_crtc_state
> > > *cstate,
> > > struct drm_plane_state *pstate;
> > > enum pipe pipe = intel_crtc->pipe;
> > > struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb;
> > > - uint16_t alloc_size, start, cursor_blocks;
> > > + uint16_t alloc_size, start;
> > > uint16_t *minimum = cstate->wm.skl.minimum_blocks;
> > > uint16_t *y_minimum = cstate->wm.skl.minimum_y_blocks;
> > > unsigned int total_data_rate;
> > > @@ -3412,12 +3395,6 @@ skl_allocate_pipe_ddb(struct
> > > intel_crtc_state
> > > *cstate,
> > > return 0;
> > > }
> > >
> > > - cursor_blocks = skl_cursor_allocation(num_active);
> > > - ddb->plane[pipe][PLANE_CURSOR].start = alloc->end -
> > > cursor_blocks;
> > > - ddb->plane[pipe][PLANE_CURSOR].end = alloc->end;
> > > -
> > > - alloc_size -= cursor_blocks;
> > > -
> > > /* 1. Allocate the mininum required blocks for each
> > > active
> > > plane */
> > > for_each_plane_in_state(state, plane, pstate, i) {
> > > intel_plane = to_intel_plane(plane);
> > > @@ -3431,17 +3408,12 @@ skl_allocate_pipe_ddb(struct
> > > intel_crtc_state
> > > *cstate,
> > > y_minimum[id] = 0;
> > > continue;
> > > }
> > > - if (plane->type == DRM_PLANE_TYPE_CURSOR) {
> > > - minimum[id] = 0;
> > > - y_minimum[id] = 0;
> > > - continue;
> > > - }
> > >
> > > minimum[id] = skl_ddb_min_alloc(pstate, 0);
> > > y_minimum[id] = skl_ddb_min_alloc(pstate, 1);
> > > }
> > >
> > > - for (i = 0; i < PLANE_CURSOR; i++) {
> > > + for (i = 0; i < I915_MAX_PLANES; i++) {
> > > alloc_size -= minimum[i];
> > > alloc_size -= y_minimum[i];
> > > }
> > > @@ -3866,26 +3838,6 @@ void skl_write_plane_wm(struct intel_crtc
> > > *intel_crtc,
> > > &ddb->y_plane[pipe][plane]);
> > > }
> > >
> > > -void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
> > > - const struct skl_plane_wm *wm,
> > > - const struct skl_ddb_allocation *ddb)
> > > -{
> > > - struct drm_crtc *crtc = &intel_crtc->base;
> > > - struct drm_device *dev = crtc->dev;
> > > - struct drm_i915_private *dev_priv = to_i915(dev);
> > > - int level, max_level = ilk_wm_max_level(dev_priv);
> > > - enum pipe pipe = intel_crtc->pipe;
> > > -
> > > - for (level = 0; level <= max_level; level++) {
> > > - skl_write_wm_level(dev_priv, CUR_WM(pipe,
> > > level),
> > > - &wm->wm[level]);
> > > - }
> > > - skl_write_wm_level(dev_priv, CUR_WM_TRANS(pipe), &wm-
> > > >
> > > > trans_wm);
> > > -
> > > - skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe),
> > > - &ddb->plane[pipe][PLANE_CURSOR]);
> > > -}
> > > -
> > > bool skl_wm_level_equals(const struct skl_wm_level *l1,
> > > const struct skl_wm_level *l2)
> > > {
> > > @@ -4122,19 +4074,11 @@ skl_print_wm_changes(const struct
> > > drm_atomic_state *state)
> > > if (skl_ddb_entry_equal(old, new))
> > > continue;
> > >
> > > - if (id != PLANE_CURSOR) {
> > > - DRM_DEBUG_ATOMIC("[PLANE:%d:plan
> > > e
> > > %d%c] ddb (%d - %d) -> (%d - %d)\n",
> > > - plane->base.id,
> > > id
> > > + 1,
> > > - pipe_name(pipe)
> > > ,
> > > - old->start,
> > > old-
> > > >
> > > > end,
> > > - new->start,
> > > new-
> > > >
> > > > end);
> > > - } else {
> > > - DRM_DEBUG_ATOMIC("[PLANE:%d:curs
> > > or
> > > %c] ddb (%d - %d) -> (%d - %d)\n",
> > > - plane->base.id,
> > > - pipe_name(pipe)
> > > ,
> > > - old->start,
> > > old-
> > > >
> > > > end,
> > > - new->start,
> > > new-
> > > >
> > > > end);
> > > - }
> > > + DRM_DEBUG_ATOMIC("[PLANE:%d:plane %d%c]
> > > ddb
> > > (%d - %d) -> (%d - %d)\n",
> > > + plane->base.id, id + 1,
> > > + pipe_name(pipe),
> > > + old->start, old->end,
> > > + new->start, new->end);
> > > }
> > > }
> > > }
> > > @@ -4235,9 +4179,6 @@ static void skl_update_wm(struct drm_crtc
> > > *crtc)
> > > for_each_universal_plane(dev_priv, pipe, plane)
> > > skl_write_plane_wm(intel_crtc, &pipe_wm-
> > > >
> > > > planes[plane],
> > > &results->ddb,
> > > plane);
> > > -
> > > - skl_write_cursor_wm(intel_crtc, &pipe_wm-
> > > >
> > > > planes[PLANE_CURSOR],
> > > - &results->ddb);
> > > }
> > >
> > > skl_copy_wm_for_pipe(hw_vals, results, pipe);
> > > @@ -4350,18 +4291,12 @@ void skl_pipe_wm_get_hw_state(struct
> > > drm_crtc
> > > *crtc,
> > > wm = &out->planes[id];
> > >
> > > for (level = 0; level <= max_level; level++) {
> > > - if (id != PLANE_CURSOR)
> > > - val = I915_READ(PLANE_WM(pipe,
> > > id,
> > > level));
> > > - else
> > > - val = I915_READ(CUR_WM(pipe,
> > > level));
> > > + val = I915_READ(PLANE_WM(pipe, id,
> > > level));
> > >
> > > skl_wm_level_from_reg_val(val, &wm-
> > > >
> > > > wm[level]);
> > > }
> > >
> > > - if (id != PLANE_CURSOR)
> > > - val = I915_READ(PLANE_WM_TRANS(pipe,
> > > id));
> > > - else
> > > - val = I915_READ(CUR_WM_TRANS(pipe));
> > > + val = I915_READ(PLANE_WM_TRANS(pipe, id));
> > >
> > > skl_wm_level_from_reg_val(val, &wm->trans_wm);
> > > }
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > > b/drivers/gpu/drm/i915/intel_sprite.c
> > > index 43d0350..9e6406a 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -194,7 +194,7 @@ void intel_pipe_update_end(struct intel_crtc
> > > *crtc, struct intel_flip_work *work
> > > }
> > > }
> > >
> > > -static void
> > > +void
> > > skl_update_plane(struct drm_plane *drm_plane,
> > > const struct intel_crtc_state *crtc_state,
> > > const struct intel_plane_state *plane_state)
> > > @@ -285,7 +285,7 @@ skl_update_plane(struct drm_plane *drm_plane,
> > > POSTING_READ(PLANE_SURF(pipe, plane));
> > > }
> > >
> > > -static void
> > > +void
> > > skl_disable_plane(struct drm_plane *dplane, struct drm_crtc
> > > *crtc)
> > > {
> > > struct drm_device *dev = dplane->dev;
> > > @@ -752,7 +752,7 @@ ilk_disable_plane(struct drm_plane *plane,
> > > struct
> > > drm_crtc *crtc)
> > > POSTING_READ(DVSSURF(pipe));
> > > }
> > >
> > > -static int
> > > +int
> > > intel_check_sprite_plane(struct drm_plane *plane,
> > > struct intel_crtc_state *crtc_state,
> > > struct intel_plane_state *state)
> > --
> > Cheers,
> > Lyude
>
--
Cheers,
Lyude
More information about the Intel-gfx
mailing list