Re: [PATCH] drm/i915/tgl: Enabling DSC on Pipe A for TGL

Manasi Navare
2019-08-15T18:51:55Z

On Thu, Aug 15, 2019 at 11:39:54AM -0700, Tolakanahalli Pradeep, Madhumitha wrote:
> On Thu, 2019-08-15 at 11:24 -0700, Manasi Navare wrote:
> > On Wed, Aug 14, 2019 at 04:51:17PM -0700, Madhumitha Tolakanahalli
> > Pradeep wrote:
> > > Removing restriction on Pipe A as TigerLake onwards, all the pipes
> > > support DSC.
> >
> > May be elaborate this commit message a little bit something like:
> > "On previous platforms, DSC was not supported on Pipe A while
> > starting TGL, its is supported
> > on all pipes. So remove the DSC and FEC restriction on Pipe A for TGL
> > onwards.
> >
>
> Alright, will update that for rev2.
>
> > >
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Signed-off-by: Madhumitha Tolakanahalli Pradeep <
> > > madhumitha.tolakanahalli.pradeep@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_dp.c | 16 ++++++++++++----
> > > 1 file changed, 12 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 4884c87c8ed7..a5b50f93fac5 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -1739,8 +1739,12 @@ static bool
> > > intel_dp_source_supports_fec(struct intel_dp *intel_dp,
> > > {
> > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > >
> > > - return INTEL_GEN(dev_priv) >= 11 &&
> > > - pipe_config->cpu_transcoder != TRANSCODER_A;
> > > + /* On TGL, DSC is supported on all Pipes */
> >
> > ^^^^ FEC supported on all pipes
>
> Oops, will change this.
>
> > > + if (INTEL_GEN(dev_priv) >= 12)
> > > + return true;
> > > + else
> > > + return INTEL_GEN(dev_priv) == 11 &&

Also here I think its better to use IS_GEN(dev_priv, 11)

> > > + pipe_config->cpu_transcoder != TRANSCODER_A;
> > > }
> > >
> > > static bool intel_dp_supports_fec(struct intel_dp *intel_dp,
> > > @@ -1755,8 +1759,12 @@ static bool
> > > intel_dp_source_supports_dsc(struct intel_dp *intel_dp,
> > > {
> > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > >
> > > - return INTEL_GEN(dev_priv) >= 10 &&
> > > - pipe_config->cpu_transcoder != TRANSCODER_A;
> > > + /* On TGL, DSC is supported on all Pipes */
> > > + if (INTEL_GEN(dev_priv) >= 12)
> > > + return true;
> > > + else
> > > + return (INTEL_GEN(dev_priv) == 10 ||
> > > INTEL_GEN(dev_priv) == 11) &&
> >
> > Why cant you just use INTEL_GEN(dev_priv) >=10 ?
>
> INTEL_GEN(dev_priv) >= 10 was the existing condition. With the new
> condition added, there would be an overlapping set
> ie INTEL_GEN(dev_priv) >= 10 would still be TRUE for GEN >= 12. Though
> this wouldn't affect the logic of the code, thought it would make more
> sense to sperate it out this way.

But since we return for GEN >=12 , the only time it would fall to GEN >=10 is for 10 and 11
so that should work, or you could use IS_GEN(dev_priv, 10) || IS_GEN(dev_priv, 11)

But may be check with Lucas on what would be the preferred way

Manasi
>
> >
> > Manasi
> >
> > > + pipe_config->cpu_transcoder != TRANSCODER_A;
> > > }
> > >
> > > static bool intel_dp_supports_dsc(struct intel_dp *intel_dp,
> > > --
> > > 2.17.1
> > >