| Message ID | 20210708102549.27821-4-vandita.kulkarni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Set BPP in the kernel | expand |
On Thu, 08 Jul 2021, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote: > Set DSC BPP to the value forced through > debugfs. It can go from bpc to bpp-1. > > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 5b52beaddada..3e50cdd7e448 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -1240,6 +1240,23 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp, > pipe_config->port_clock = intel_dp->common_rates[limits->max_clock]; > pipe_config->lane_count = limits->max_lane_count; > > + if (intel_dp->force_dsc_en) { > + /* As of today we support DSC for only RGB */ > + if (intel_dp->force_dsc_bpp >= 8 && > + intel_dp->force_dsc_bpp < pipe_bpp) { > + drm_dbg_kms(&dev_priv->drm, > + "DSC BPP forced to %d", > + intel_dp->force_dsc_bpp); > + pipe_config->dsc.compressed_bpp = > + intel_dp->force_dsc_bpp; > + } else { > + drm_dbg_kms(&dev_priv->drm, > + "Invalid DSC BPP %d", > + intel_dp->force_dsc_bpp); > + return -EINVAL; I'd just let it use the normal compressed_bpp, with the debug message, instead of returning -EINVAL. > + } > + } > + This should be *after* the below blocks, because otherwise compressed_bpp will be overridden by the normal case, not by the force case! BR, Jani. > if (intel_dp_is_edp(intel_dp)) { > pipe_config->dsc.compressed_bpp = > min_t(u16, drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4,
On Thu, 08 Jul 2021, Jani Nikula <jani.nikula@intel.com> wrote: > On Thu, 08 Jul 2021, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote: >> Set DSC BPP to the value forced through >> debugfs. It can go from bpc to bpp-1. >> >> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c >> index 5b52beaddada..3e50cdd7e448 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dp.c >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c >> @@ -1240,6 +1240,23 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp, >> pipe_config->port_clock = intel_dp->common_rates[limits->max_clock]; >> pipe_config->lane_count = limits->max_lane_count; >> >> + if (intel_dp->force_dsc_en) { Oh, this should check for intel_dp->force_dsc_bpp. We don't want to always force the bpp when we force dsc enable. >> + /* As of today we support DSC for only RGB */ >> + if (intel_dp->force_dsc_bpp >= 8 && >> + intel_dp->force_dsc_bpp < pipe_bpp) { >> + drm_dbg_kms(&dev_priv->drm, >> + "DSC BPP forced to %d", >> + intel_dp->force_dsc_bpp); >> + pipe_config->dsc.compressed_bpp = >> + intel_dp->force_dsc_bpp; >> + } else { >> + drm_dbg_kms(&dev_priv->drm, >> + "Invalid DSC BPP %d", >> + intel_dp->force_dsc_bpp); >> + return -EINVAL; > > I'd just let it use the normal compressed_bpp, with the debug message, > instead of returning -EINVAL. > >> + } >> + } >> + > > This should be *after* the below blocks, because otherwise > compressed_bpp will be overridden by the normal case, not by the force > case! > > BR, > Jani. > >> if (intel_dp_is_edp(intel_dp)) { >> pipe_config->dsc.compressed_bpp = >> min_t(u16, drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4,
> -----Original Message----- > From: Nikula, Jani <jani.nikula@intel.com> > Sent: Thursday, July 8, 2021 6:44 PM > To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Kulkarni, Vandita <vandita.kulkarni@intel.com> > Subject: Re: [v7 3/3] drm/i915/display/dsc: Force dsc BPP > > On Thu, 08 Jul 2021, Jani Nikula <jani.nikula@intel.com> wrote: > > On Thu, 08 Jul 2021, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote: > >> Set DSC BPP to the value forced through debugfs. It can go from bpc > >> to bpp-1. > >> > >> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com> > >> --- > >> drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++ > >> 1 file changed, 17 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > >> b/drivers/gpu/drm/i915/display/intel_dp.c > >> index 5b52beaddada..3e50cdd7e448 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_dp.c > >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c > >> @@ -1240,6 +1240,23 @@ static int intel_dp_dsc_compute_config(struct > intel_dp *intel_dp, > >> pipe_config->port_clock = intel_dp->common_rates[limits- > >max_clock]; > >> pipe_config->lane_count = limits->max_lane_count; > >> > >> + if (intel_dp->force_dsc_en) { > > Oh, this should check for intel_dp->force_dsc_bpp. We don't want to always > force the bpp when we force dsc enable. Okay will fix this. And I was returning -EINVAL , to fail the test on setting invalid BPP. > > >> + /* As of today we support DSC for only RGB */ > >> + if (intel_dp->force_dsc_bpp >= 8 && > >> + intel_dp->force_dsc_bpp < pipe_bpp) { > >> + drm_dbg_kms(&dev_priv->drm, > >> + "DSC BPP forced to %d", > >> + intel_dp->force_dsc_bpp); > >> + pipe_config->dsc.compressed_bpp = > >> + intel_dp->force_dsc_bpp; > >> + } else { > >> + drm_dbg_kms(&dev_priv->drm, > >> + "Invalid DSC BPP %d", > >> + intel_dp->force_dsc_bpp); > >> + return -EINVAL; > > > > I'd just let it use the normal compressed_bpp, with the debug message, > > instead of returning -EINVAL. > > > >> + } > >> + } > >> + > > > > This should be *after* the below blocks, because otherwise > > compressed_bpp will be overridden by the normal case, not by the force > > case! > > > > BR, > > Jani. > > > >> if (intel_dp_is_edp(intel_dp)) { > >> pipe_config->dsc.compressed_bpp = > >> min_t(u16, > drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4, > > -- > Jani Nikula, Intel Open Source Graphics Center
On Thu, 08 Jul 2021, "Kulkarni, Vandita" <vandita.kulkarni@intel.com> wrote: >> -----Original Message----- >> From: Nikula, Jani <jani.nikula@intel.com> >> Sent: Thursday, July 8, 2021 6:44 PM >> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel- >> gfx@lists.freedesktop.org >> Cc: Kulkarni, Vandita <vandita.kulkarni@intel.com> >> Subject: Re: [v7 3/3] drm/i915/display/dsc: Force dsc BPP >> >> On Thu, 08 Jul 2021, Jani Nikula <jani.nikula@intel.com> wrote: >> > On Thu, 08 Jul 2021, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote: >> >> Set DSC BPP to the value forced through debugfs. It can go from bpc >> >> to bpp-1. >> >> >> >> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com> >> >> --- >> >> drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++ >> >> 1 file changed, 17 insertions(+) >> >> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c >> >> b/drivers/gpu/drm/i915/display/intel_dp.c >> >> index 5b52beaddada..3e50cdd7e448 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c >> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c >> >> @@ -1240,6 +1240,23 @@ static int intel_dp_dsc_compute_config(struct >> intel_dp *intel_dp, >> >> pipe_config->port_clock = intel_dp->common_rates[limits- >> >max_clock]; >> >> pipe_config->lane_count = limits->max_lane_count; >> >> >> >> + if (intel_dp->force_dsc_en) { >> >> Oh, this should check for intel_dp->force_dsc_bpp. We don't want to always >> force the bpp when we force dsc enable. > Okay will fix this. > And I was returning -EINVAL , to fail the test on setting invalid BPP. Okay, if it makes the test easier, I guess it's fine. Up to you. BR, Jani. > >> >> >> + /* As of today we support DSC for only RGB */ >> >> + if (intel_dp->force_dsc_bpp >= 8 && >> >> + intel_dp->force_dsc_bpp < pipe_bpp) { >> >> + drm_dbg_kms(&dev_priv->drm, >> >> + "DSC BPP forced to %d", >> >> + intel_dp->force_dsc_bpp); >> >> + pipe_config->dsc.compressed_bpp = >> >> + intel_dp->force_dsc_bpp; >> >> + } else { >> >> + drm_dbg_kms(&dev_priv->drm, >> >> + "Invalid DSC BPP %d", >> >> + intel_dp->force_dsc_bpp); >> >> + return -EINVAL; >> > >> > I'd just let it use the normal compressed_bpp, with the debug message, >> > instead of returning -EINVAL. >> > >> >> + } >> >> + } >> >> + >> > >> > This should be *after* the below blocks, because otherwise >> > compressed_bpp will be overridden by the normal case, not by the force >> > case! >> > >> > BR, >> > Jani. >> > >> >> if (intel_dp_is_edp(intel_dp)) { >> >> pipe_config->dsc.compressed_bpp = >> >> min_t(u16, >> drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4, >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
> -----Original Message----- > From: Nikula, Jani <jani.nikula@intel.com> > Sent: Thursday, July 8, 2021 9:53 PM > To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel- > gfx@lists.freedesktop.org > Subject: RE: [v7 3/3] drm/i915/display/dsc: Force dsc BPP > > On Thu, 08 Jul 2021, "Kulkarni, Vandita" <vandita.kulkarni@intel.com> wrote: > >> -----Original Message----- > >> From: Nikula, Jani <jani.nikula@intel.com> > >> Sent: Thursday, July 8, 2021 6:44 PM > >> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel- > >> gfx@lists.freedesktop.org > >> Cc: Kulkarni, Vandita <vandita.kulkarni@intel.com> > >> Subject: Re: [v7 3/3] drm/i915/display/dsc: Force dsc BPP > >> > >> On Thu, 08 Jul 2021, Jani Nikula <jani.nikula@intel.com> wrote: > >> > On Thu, 08 Jul 2021, Vandita Kulkarni <vandita.kulkarni@intel.com> > wrote: > >> >> Set DSC BPP to the value forced through debugfs. It can go from > >> >> bpc to bpp-1. > >> >> > >> >> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com> > >> >> --- > >> >> drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++ > >> >> 1 file changed, 17 insertions(+) > >> >> > >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > >> >> b/drivers/gpu/drm/i915/display/intel_dp.c > >> >> index 5b52beaddada..3e50cdd7e448 100644 > >> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c > >> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c > >> >> @@ -1240,6 +1240,23 @@ static int > >> >> intel_dp_dsc_compute_config(struct > >> intel_dp *intel_dp, > >> >> pipe_config->port_clock = intel_dp->common_rates[limits- > >> >max_clock]; > >> >> pipe_config->lane_count = limits->max_lane_count; > >> >> > >> >> + if (intel_dp->force_dsc_en) { > >> > >> Oh, this should check for intel_dp->force_dsc_bpp. We don't want to > >> always force the bpp when we force dsc enable. > > Okay will fix this. > > And I was returning -EINVAL , to fail the test on setting invalid BPP. > > Okay, if it makes the test easier, I guess it's fine. Up to you. Okay, for now I have sent a patch like you suggested, as I see that there are no negative test cases . Have sent the v2 of this patch. So, it wouldn't make much difference. Thanks, Vandita > > BR, > Jani. > > > > >> > >> >> + /* As of today we support DSC for only RGB */ > >> >> + if (intel_dp->force_dsc_bpp >= 8 && > >> >> + intel_dp->force_dsc_bpp < pipe_bpp) { > >> >> + drm_dbg_kms(&dev_priv->drm, > >> >> + "DSC BPP forced to %d", > >> >> + intel_dp->force_dsc_bpp); > >> >> + pipe_config->dsc.compressed_bpp = > >> >> + intel_dp->force_dsc_bpp; > >> >> + } else { > >> >> + drm_dbg_kms(&dev_priv->drm, > >> >> + "Invalid DSC BPP %d", > >> >> + intel_dp->force_dsc_bpp); > >> >> + return -EINVAL; > >> > > >> > I'd just let it use the normal compressed_bpp, with the debug > >> > message, instead of returning -EINVAL. > >> > > >> >> + } > >> >> + } > >> >> + > >> > > >> > This should be *after* the below blocks, because otherwise > >> > compressed_bpp will be overridden by the normal case, not by the > >> > force case! > >> > > >> > BR, > >> > Jani. > >> > > >> >> if (intel_dp_is_edp(intel_dp)) { > >> >> pipe_config->dsc.compressed_bpp = > >> >> min_t(u16, > >> drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4, > >> > >> -- > >> Jani Nikula, Intel Open Source Graphics Center > > -- > Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 5b52beaddada..3e50cdd7e448 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -1240,6 +1240,23 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp, pipe_config->port_clock = intel_dp->common_rates[limits->max_clock]; pipe_config->lane_count = limits->max_lane_count; + if (intel_dp->force_dsc_en) { + /* As of today we support DSC for only RGB */ + if (intel_dp->force_dsc_bpp >= 8 && + intel_dp->force_dsc_bpp < pipe_bpp) { + drm_dbg_kms(&dev_priv->drm, + "DSC BPP forced to %d", + intel_dp->force_dsc_bpp); + pipe_config->dsc.compressed_bpp = + intel_dp->force_dsc_bpp; + } else { + drm_dbg_kms(&dev_priv->drm, + "Invalid DSC BPP %d", + intel_dp->force_dsc_bpp); + return -EINVAL; + } + } + if (intel_dp_is_edp(intel_dp)) { pipe_config->dsc.compressed_bpp = min_t(u16, drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4,
Set DSC BPP to the value forced through debugfs. It can go from bpc to bpp-1. Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com> --- drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)