Re: [Intel-gfx] [PATCH v2 02/22] drm/i915/dp: Track the pipe and link bpp limits separately

[Imre Deak <imre.deak@intel.com>]

On Thu, Sep 14, 2023 at 12:33:59PM +0300, Luca Coelho wrote:
> On Thu, 2023-08-24 at 11:04 +0300, Imre Deak wrote:
> > A follow-up patch will need to limit the output link bpp both in the
> > non-DSC and DSC configuration, so track the pipe and link bpp limits
> > separately in the link_config_limits struct.
> > 
> > Use .4 fixed point format for link bpp matching the 1/16 bpp granularity
> > in DSC mode and for now keep this limit matching the pipe bpp limit.
> > 
> > v2: (Jani)
> > - Add to_bpp_int(), to_bpp_x16() helpers instead of opencoding them.
> > - Rename link_config_limits::link.min/max_bpp to min/max_bpp_x16.
> > 
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_types.h    | 10 ++++++++
> >  drivers/gpu/drm/i915/display/intel_dp.c       | 25 +++++++++++--------
> >  drivers/gpu/drm/i915/display/intel_dp.h       |  9 ++++++-
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c   | 17 ++++++++-----
> >  4 files changed, 44 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 731f2ec04d5cd..5875eff5012ce 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> 
> [...]
> 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> > index 788a577ebe16e..ebc7f4e60c777 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > @@ -26,7 +26,14 @@ struct intel_encoder;
> >  struct link_config_limits {
> >  	int min_rate, max_rate;
> >  	int min_lane_count, max_lane_count;
> > -	int min_bpp, max_bpp;
> > +	struct {
> > +		/* Uncompressed DSC input or link output bpp in 1 bpp units */
> > +		int min_bpp, max_bpp;
> > +	} pipe;
> > +	struct {
> > +		/* Compressed or uncompressed link output bpp in 1/16 bpp units */
> > +		int min_bpp_x16, max_bpp_x16;
> > +	} link;
> >  };
> 
> It's not clear to me from the commit message (nor from the code, for
> that matter) why you need to store the values in both formats.  Can you
> clarify?

For DSC configuration two separate limits need to be considered:

One is the bpp value which is a property of the pixel format input to
the DSC engine, for this the DSC state computation should use the
pipe.min/max_bpp limits and this functionality of the DSC HW block can
be configured in 1 bits per pixel granularity.

The other one is the bpp value which is the format of pixels output from
the DSC engine (and is the actual pixel format on the link), for which
the DSC state computation should use link.min/max_bpp_x16. The DSC HW
block can be configure this pixel format in 1/16 bits per granularity.

For instance pipe.min/max_bpp will be 16 .. 30 bpp range (in 1 bpp
units), link.min/max_bpp_x16 in the 8 .. 27 bpp range (in 1/16 bpp
units).

> 
> --
> Cheers,
> Luca.