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

[Manasi Navare <manasi.d.navare@intel.com>]

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
> > > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx