| Message ID | 20190814235117.11304-1-madhumitha.tolakanahalli.pradeep@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/tgl: Enabling DSC on Pipe A for TGL | expand |
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. > > 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 > + if (INTEL_GEN(dev_priv) >= 12) > + return true; > + else > + return INTEL_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 ? Manasi > + pipe_config->cpu_transcoder != TRANSCODER_A; > } > > static bool intel_dp_supports_dsc(struct intel_dp *intel_dp, > -- > 2.17.1 >
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 && > > + 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. > > Manasi > > > + pipe_config->cpu_transcoder != TRANSCODER_A; > > } > > > > static bool intel_dp_supports_dsc(struct intel_dp *intel_dp, > > -- > > 2.17.1 > >
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 > > >
On Thu, 2019-08-15 at 11:53 -0700, Manasi Navare wrote: > 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) Yes, that does make it clearer, I'll change it for v2. > > > > > + 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 Yeah, it wouldn't affect the logic. I debated about it for a while too. @Lucas, what do you think is the preferred way to implement this? > > Manasi > > > > > > > > Manasi > > > > > > > + pipe_config->cpu_transcoder != > > > > TRANSCODER_A; > > > > } > > > > > > > > static bool intel_dp_supports_dsc(struct intel_dp *intel_dp, > > > > -- > > > > 2.17.1 > > > >
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 */ + if (INTEL_GEN(dev_priv) >= 12) + return true; + else + return INTEL_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) && + pipe_config->cpu_transcoder != TRANSCODER_A; } static bool intel_dp_supports_dsc(struct intel_dp *intel_dp,
Removing restriction on Pipe A as TigerLake onwards, all the pipes support DSC. 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(-)