| Message ID | 20230626082821.2085-1-stanislav.lisovskiy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Fail if DSC compression requirement is less than platform supports | expand |
On Mon, 2023-06-26 at 11:28 +0300, Stanislav Lisovskiy wrote: > Currently we just clamp that value to the highest supported one, however that > means, we are not able to fit this into our available bandwidth range, so we > might see glitches or FIFO underruns. > While choosing less compressed bpp than min bpp required to handle the mode is > harmless and might even save some bandwidth, choosing higher compressed bpp than > min bpp required to handle the required mode config, can cause issues. > So in that case lets just conclude that even with DSC, we are not able to comply > with bandwidth requirements and fail. > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 9f40da20e88d..8696a1f02805 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -713,8 +713,17 @@ u32 intel_dp_dsc_nearest_valid_bpp(struct drm_i915_private *i915, u32 bpp, u32 p > > /* > * According to BSpec, 27 is the max DSC output bpp, > - * 8 is the min DSC output bpp > + * 8 is the min DSC output bpp. > + * While we can still clamp higher bpp values to 27, saving bandwidth, > + * if it is required to oompress up to bpp < 8, means we can't do Small typo, "compress". > + * that and probably means we can't fit the required mode, even with > + * DSC enabled. > */ > + if (bits_per_pixel < 8) { > + drm_dbg_kms(&i915->drm, "Unsupported BPP %u, min 8\n", > + bits_per_pixel); > + return 0; > + } > bits_per_pixel = clamp_t(u32, bits_per_pixel, 8, 27); I guess you don't need to clamp anymore but could use min_t(u32, bits_per_pixel, 27) now, right? Actually, you don't even need to type it, so min(bits_per_pixel, 27) should suffice. > } else { > /* Find the nearest match in the array of known BPPs from VESA */ -- Cheers, Luca.
On Mon, Jun 26, 2023 at 03:18:52PM +0300, Luca Coelho wrote: > On Mon, 2023-06-26 at 11:28 +0300, Stanislav Lisovskiy wrote: > > Currently we just clamp that value to the highest supported one, however that > > means, we are not able to fit this into our available bandwidth range, so we > > might see glitches or FIFO underruns. > > While choosing less compressed bpp than min bpp required to handle the mode is > > harmless and might even save some bandwidth, choosing higher compressed bpp than > > min bpp required to handle the required mode config, can cause issues. > > So in that case lets just conclude that even with DSC, we are not able to comply > > with bandwidth requirements and fail. > > > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_dp.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > index 9f40da20e88d..8696a1f02805 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -713,8 +713,17 @@ u32 intel_dp_dsc_nearest_valid_bpp(struct drm_i915_private *i915, u32 bpp, u32 p > > > > /* > > * According to BSpec, 27 is the max DSC output bpp, > > - * 8 is the min DSC output bpp > > + * 8 is the min DSC output bpp. > > + * While we can still clamp higher bpp values to 27, saving bandwidth, > > + * if it is required to oompress up to bpp < 8, means we can't do > > Small typo, "compress". Thanks for spotting > > > > + * that and probably means we can't fit the required mode, even with > > + * DSC enabled. > > */ > > + if (bits_per_pixel < 8) { > > + drm_dbg_kms(&i915->drm, "Unsupported BPP %u, min 8\n", > > + bits_per_pixel); > > + return 0; > > + } > > bits_per_pixel = clamp_t(u32, bits_per_pixel, 8, 27); > > I guess you don't need to clamp anymore but could use min_t(u32, > bits_per_pixel, 27) now, right? Actually, you don't even need to type > it, so min(bits_per_pixel, 27) should suffice. Yep, was thinking about that, was willing to leave clamp just as an additional check "just in case", but probably you are right. Stan > > > > } else { > > /* Find the nearest match in the array of known BPPs from VESA */ > > -- > Cheers, > Luca.
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 9f40da20e88d..8696a1f02805 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -713,8 +713,17 @@ u32 intel_dp_dsc_nearest_valid_bpp(struct drm_i915_private *i915, u32 bpp, u32 p /* * According to BSpec, 27 is the max DSC output bpp, - * 8 is the min DSC output bpp + * 8 is the min DSC output bpp. + * While we can still clamp higher bpp values to 27, saving bandwidth, + * if it is required to oompress up to bpp < 8, means we can't do + * that and probably means we can't fit the required mode, even with + * DSC enabled. */ + if (bits_per_pixel < 8) { + drm_dbg_kms(&i915->drm, "Unsupported BPP %u, min 8\n", + bits_per_pixel); + return 0; + } bits_per_pixel = clamp_t(u32, bits_per_pixel, 8, 27); } else { /* Find the nearest match in the array of known BPPs from VESA */
Currently we just clamp that value to the highest supported one, however that means, we are not able to fit this into our available bandwidth range, so we might see glitches or FIFO underruns. While choosing less compressed bpp than min bpp required to handle the mode is harmless and might even save some bandwidth, choosing higher compressed bpp than min bpp required to handle the required mode config, can cause issues. So in that case lets just conclude that even with DSC, we are not able to comply with bandwidth requirements and fail. Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> --- drivers/gpu/drm/i915/display/intel_dp.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)