| Message ID | 20201104195604.3334-2-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Re-enable FBC on TGL | expand |
On 2020-11-05 at 01:26:03 +0530, Uma Shankar wrote: > There are some corner cases wrt underrun when we enable > FBC with PSR2 on TGL. Recommendation from hardware is to > keep this combination disabled. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c > index a5b072816a7b..32c411414908 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > @@ -799,6 +799,12 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) > struct intel_fbc *fbc = &dev_priv->fbc; > struct intel_fbc_state_cache *cache = &fbc->state_cache; > > + if (dev_priv->psr.sink_psr2_support && > + IS_TIGERLAKE(dev_priv)) { IMHO we need to use state boolean crtc_state->has_psr2, we can have sink supports PSR2 but it may not be enabled due to any reason. Thanks, Anshuman Gupta. > + fbc->no_fbc_reason = "not supported with PSR2"; > + return false; > + } > + > if (!intel_fbc_can_enable(dev_priv)) > return false; > > -- > 2.26.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> -----Original Message----- > From: Anshuman Gupta <anshuman.gupta@intel.com> > Sent: Thursday, November 5, 2020 12:12 PM > To: Shankar, Uma <uma.shankar@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/display/tgl: Disable FBC with PSR2 > > On 2020-11-05 at 01:26:03 +0530, Uma Shankar wrote: > > There are some corner cases wrt underrun when we enable FBC with PSR2 > > on TGL. Recommendation from hardware is to keep this combination > > disabled. > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c > > b/drivers/gpu/drm/i915/display/intel_fbc.c > > index a5b072816a7b..32c411414908 100644 > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > > @@ -799,6 +799,12 @@ static bool intel_fbc_can_activate(struct intel_crtc > *crtc) > > struct intel_fbc *fbc = &dev_priv->fbc; > > struct intel_fbc_state_cache *cache = &fbc->state_cache; > > > > + if (dev_priv->psr.sink_psr2_support && > > + IS_TIGERLAKE(dev_priv)) { > IMHO we need to use state boolean crtc_state->has_psr2, we can have sink > supports PSR2 but it may not be enabled due to any reason. Yeah agree, will update this check. Thanks. Regards, Uma Shankar > Thanks, > Anshuman Gupta. > > + fbc->no_fbc_reason = "not supported with PSR2"; > > + return false; > > + } > > + > > if (!intel_fbc_can_enable(dev_priv)) > > return false; > > > > -- > > 2.26.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 2020-11-05 at 01:26 +0530, Uma Shankar wrote: > There are some corner cases wrt underrun when we enable > FBC with PSR2 on TGL. Recommendation from hardware is to > keep this combination disabled. Do you have any references to this? HSD? BSpec? > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c > index a5b072816a7b..32c411414908 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > @@ -799,6 +799,12 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) > struct intel_fbc *fbc = &dev_priv->fbc; > struct intel_fbc_state_cache *cache = &fbc->state_cache; > > > > > + if (dev_priv->psr.sink_psr2_support && > + IS_TIGERLAKE(dev_priv)) { > + fbc->no_fbc_reason = "not supported with PSR2"; > + return false; > + } > + > if (!intel_fbc_can_enable(dev_priv)) > return false; > > > >
> -----Original Message----- > From: Souza, Jose <jose.souza@intel.com> > Sent: Thursday, November 5, 2020 9:38 PM > To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/display/tgl: Disable FBC with PSR2 > > On Thu, 2020-11-05 at 01:26 +0530, Uma Shankar wrote: > > There are some corner cases wrt underrun when we enable FBC with PSR2 > > on TGL. Recommendation from hardware is to keep this combination > > disabled. > > Do you have any references to this? HSD? BSpec? Hi Jose, Below is the HSD for the same: https://hsdes.intel.com/appstore/article/#/14010260002 Will add the link in patch as well. Regards, Uma Shankar > > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c > > b/drivers/gpu/drm/i915/display/intel_fbc.c > > index a5b072816a7b..32c411414908 100644 > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > > @@ -799,6 +799,12 @@ static bool intel_fbc_can_activate(struct intel_crtc > *crtc) > > struct intel_fbc *fbc = &dev_priv->fbc; > > struct intel_fbc_state_cache *cache = &fbc->state_cache; > > > > > > > > > > + if (dev_priv->psr.sink_psr2_support && > > + IS_TIGERLAKE(dev_priv)) { > > + fbc->no_fbc_reason = "not supported with PSR2"; > > + return false; > > + } > > + > > if (!intel_fbc_can_enable(dev_priv)) > > return false; > > > > > > > >
On Thu, 2020-11-05 at 21:57 +0530, Shankar, Uma wrote: > > > -----Original Message----- > > From: Souza, Jose <jose.souza@intel.com> > > Sent: Thursday, November 5, 2020 9:38 PM > > To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/display/tgl: Disable FBC with PSR2 > > > > On Thu, 2020-11-05 at 01:26 +0530, Uma Shankar wrote: > > > There are some corner cases wrt underrun when we enable FBC with PSR2 > > > on TGL. Recommendation from hardware is to keep this combination > > > disabled. > > > > Do you have any references to this? HSD? BSpec? > > Hi Jose, > Below is the HSD for the same: > https://hsdes.intel.com/appstore/article/#/14010260002 > > Will add the link in patch as well. I have commented in that HSD in the past, it looked to me that we were not affected by that as that HSD refers to GEN11+. Also that HSD looks odd there is no real report of issue there. Are you sure that the FBC underruns are because of PSR2? Not all TGL systems in CI have a PSR2 panel, please make sure we are not disabling FBC in vain. > > Regards, > Uma Shankar > > > > > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c > > > b/drivers/gpu/drm/i915/display/intel_fbc.c > > > index a5b072816a7b..32c411414908 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > > > @@ -799,6 +799,12 @@ static bool intel_fbc_can_activate(struct intel_crtc > > *crtc) > > > struct intel_fbc *fbc = &dev_priv->fbc; > > > struct intel_fbc_state_cache *cache = &fbc->state_cache; > > > > > > > > > > > > > > > + if (dev_priv->psr.sink_psr2_support && > > > + IS_TIGERLAKE(dev_priv)) { > > > + fbc->no_fbc_reason = "not supported with PSR2"; > > > + return false; > > > + } > > > + > > > if (!intel_fbc_can_enable(dev_priv)) > > > return false; > > > > > > > > > > > > >
> -----Original Message----- > From: Souza, Jose <jose.souza@intel.com> > Sent: Thursday, November 5, 2020 11:09 PM > To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/display/tgl: Disable FBC with PSR2 > > On Thu, 2020-11-05 at 21:57 +0530, Shankar, Uma wrote: > > > > > -----Original Message----- > > > From: Souza, Jose <jose.souza@intel.com> > > > Sent: Thursday, November 5, 2020 9:38 PM > > > To: Shankar, Uma <uma.shankar@intel.com>; > > > intel-gfx@lists.freedesktop.org > > > Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/display/tgl: Disable > > > FBC with PSR2 > > > > > > On Thu, 2020-11-05 at 01:26 +0530, Uma Shankar wrote: > > > > There are some corner cases wrt underrun when we enable FBC with > > > > PSR2 on TGL. Recommendation from hardware is to keep this > > > > combination disabled. > > > > > > Do you have any references to this? HSD? BSpec? > > > > Hi Jose, > > Below is the HSD for the same: > > https://hsdes.intel.com/appstore/article/#/14010260002 > > > > Will add the link in patch as well. > > I have commented in that HSD in the past, it looked to me that we were not > affected by that as that HSD refers to GEN11+. Also that HSD looks odd there is > no real report of issue there. > > Are you sure that the FBC underruns are because of PSR2? Not all TGL systems in > CI have a PSR2 panel, please make sure we are not disabling FBC in vain. Yeah we had similar concern and raised it up with hardware design team. They are taking it forward to get this officially documented in spec. Forwarded you an offline internal mail regarding this. > > > > Regards, > > Uma Shankar > > > > > > > > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++++++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c > > > > b/drivers/gpu/drm/i915/display/intel_fbc.c > > > > index a5b072816a7b..32c411414908 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > > > > @@ -799,6 +799,12 @@ static bool intel_fbc_can_activate(struct > > > > intel_crtc > > > *crtc) > > > > struct intel_fbc *fbc = &dev_priv->fbc; > > > > struct intel_fbc_state_cache *cache = &fbc->state_cache; > > > > > > > > > > > > > > > > > > > > + if (dev_priv->psr.sink_psr2_support && > > > > + IS_TIGERLAKE(dev_priv)) { > > > > + fbc->no_fbc_reason = "not supported with PSR2"; > > > > + return false; > > > > + } > > > > + > > > > if (!intel_fbc_can_enable(dev_priv)) > > > > return false; > > > > > > > > > > > > > > > > > >
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index a5b072816a7b..32c411414908 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -799,6 +799,12 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) struct intel_fbc *fbc = &dev_priv->fbc; struct intel_fbc_state_cache *cache = &fbc->state_cache; + if (dev_priv->psr.sink_psr2_support && + IS_TIGERLAKE(dev_priv)) { + fbc->no_fbc_reason = "not supported with PSR2"; + return false; + } + if (!intel_fbc_can_enable(dev_priv)) return false;
There are some corner cases wrt underrun when we enable FBC with PSR2 on TGL. Recommendation from hardware is to keep this combination disabled. Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++++++ 1 file changed, 6 insertions(+)