Re: [PATCH] drm/i915: Don't use the second dbuf slice on icl

From: Mahesh Kumar <mahesh1.sh.kumar@gmail.com>
To: imre.deak@intel.com
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Don't use the second dbuf slice on icl
Date: Mon, 28 Jan 2019 09:11:18 +0530	[thread overview]
Message-ID: <CAGrStEquHQGWKFxbXC=7-gj47jxRk_GoRdKqDoRRcuMjvvqw3g@mail.gmail.com> (raw)
In-Reply-To: <20190125150833.GB12107@ideak-desk.fi.intel.com>

Hi,

On Fri, Jan 25, 2019 at 8:38 PM Imre Deak <imre.deak@intel.com> wrote:
>
> On Tue, Jan 22, 2019 at 02:49:13PM +0530, Mahesh Kumar wrote:
> > Hi,
> >
> >
> > On Mon, Jan 21, 2019 at 9:01 PM Ville Syrjala
> > <ville.syrjala@linux.intel.com> wrote:
> > &gt;
> > &gt; From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > &gt;
> > &gt; The code managing the dbuf slices is borked and needs some
> > &gt; real work to fix. In the meantime let's just stop using the
> > &gt; second slice.
> > &gt;
> > &gt; Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > &gt; ---
> > &gt;  drivers/gpu/drm/i915/intel_pm.c | 10 ++++++++--
> > &gt;  1 file changed, 8 insertions(+), 2 deletions(-)
> > &gt;
> > &gt; diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > &gt; index 8b63afa3a221..1e41c899ffe2 100644
> > &gt; --- a/drivers/gpu/drm/i915/intel_pm.c
> > &gt; +++ b/drivers/gpu/drm/i915/intel_pm.c
> > &gt; @@ -3618,7 +3618,8 @@ static u8
> > intel_enabled_dbuf_slices_num(struct drm_i915_private *dev_priv)
> > &gt;         enabled_slices = 1;
> > &gt;
> > &gt;         /* Gen prior to GEN11 have only one DBuf slice */
> > &gt; -       if (INTEL_GEN(dev_priv) &lt; 11)
> > &gt; +       /* FIXME dbuf slice code is broken: see intel_get_ddb_size() */
> > &gt; +       if (1 || INTEL_GEN(dev_priv) &lt; 11)
> > &gt;                 return enabled_slices;
> >
> > IMHO we may not need this,
>
> It's easier to pretend that we have only a single slice. Not sure for
> instance if the lack of the above would lead to state check errors (due
> to other problems in the dbuf silce implementation).
>
> > If we return from above we'll never disable
> > second slice in case it's enabled by bios.
>
> We'll disable whenever we need to, that is during suspend/driver unload.
> We can ignore the rest of the cases I think.

exactly, It will get disabled during suspend/unload, but as we are not
using second slice, it's going to consume some power until then.
This function is to tell us H/W state, So we should assume that second
slice is disabled without checking H/W state.

~Mahesh

>
> > Anyhow code change in intel_get_ddb_size will take care of enabling
> > only one slice.
> >
> > &gt;
> > &gt;         if (I915_READ(DBUF_CTL_S2) &amp; DBUF_POWER_STATE)
> > &gt; @@ -3827,8 +3828,13 @@ static u16 intel_get_ddb_size(struct
> > drm_i915_private *dev_priv,
> > &gt;
> > &gt;         /*
> > &gt;          * 12GB/s is maximum BW supported by single DBuf slice.
> > &gt; +        *
> > &gt; +        * FIXME dbuf slice code is broken:
> > &gt; +        * - must wait for planes to stop using the slice before
> > powering it off
> >
> > AFAIR we were already doing it and disabling slice only after
> > update_crtcs, and skl_update_crtc code is taking care of waiting for
> > vblank in case it's required.
> >
> > &gt; +        * - plane straddling both slices is illegal in
> > multi-pipe scenarios
> >
> > This is something new :)
> >
> > although this change introduce a major limitation with number and size
> > of planes we can display, yet
> > As code is broken and mentioned conditions need to be taken care of,
> > This change should be ok until proper fix.
> >
> > ~Mahesh
> >
> > > +        * - should validate we stay within the hw bandwidth limits
> > >          */
> > > -       if (num_active > 1 || total_data_bw >= GBps(12)) {
> > > +       if (0 && (num_active > 1 || total_data_bw >= GBps(12))) {
> > >                 ddb->enabled_slices = 2;
> > >         } else {
> > >                 ddb->enabled_slices = 1;
> > > --
> > > 2.19.2
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx