[Intel-gfx] [PATCH v2 09/22] drm/dp_mst: Fix fractional bpp scaling in drm_dp_calc_pbn_mode()

[Imre Deak]

On Mon, Sep 04, 2023 at 05:53:11AM +0300, Ville Syrjälä wrote:
> On Thu, Aug 24, 2023 at 11:05:04AM +0300, Imre Deak wrote:
> > For fractional bpp values passed to the function in a .4 fixed point
> > format, the fractional part is currently ignored due to scaling bpp too
> > early. Fix this by scaling the overhead factor instead and to avoid an
> > overflow multiplying bpp with the overhead factor instead of the clock
> > rate.
> > 
> > While at it simplify the formula, and pass the expected fixed point bpp
> > values in the kunit tests.
> > 
> > Cc: Lyude Paul <lyude at redhat.com>
> > Cc: dri-devel at lists.freedesktop.org
> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > ---
> >  drivers/gpu/drm/display/drm_dp_mst_topology.c  | 7 ++-----
> >  drivers/gpu/drm/tests/drm_dp_mst_helper_test.c | 8 ++++----
> >  2 files changed, 6 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > index ed96cfcfa3040..bd0f35a0ea5fb 100644
> > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > @@ -4712,12 +4712,9 @@ int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc)
> >  	 * factor in the numerator rather than the denominator to avoid
> >  	 * integer overflow
> >  	 */
> > +	u32 bpp_m = (dsc ? 64 / 16 : 64) * 1006 * bpp;
> >  
> > -	if (dsc)
> > -		return DIV_ROUND_UP_ULL(mul_u32_u32(clock * (bpp / 16), 64 * 1006),
> > -					8 * 54 * 1000 * 1000);
> > -
> > -	return DIV_ROUND_UP_ULL(mul_u32_u32(clock * bpp, 64 * 1006),
> > +	return DIV_ROUND_UP_ULL(mul_u32_u32(clock, bpp_m),
> >  				8 * 54 * 1000 * 1000);
> 
> I thought I sorted out this mess already...
> https://patchwork.freedesktop.org/patch/535005/?series=117201&rev=3
> Apparently I forgot to push that.

Looks ok, can use that instead. I thought clock * bpp could overflow,
but probably not in practice.

The test cases below would still need to be fixed.

> 
> >  }
> >  EXPORT_SYMBOL(drm_dp_calc_pbn_mode);
> > diff --git a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
> > index 545beea33e8c7..ea2182815ebe8 100644
> > --- a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
> > +++ b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
> > @@ -40,15 +40,15 @@ static const struct drm_dp_mst_calc_pbn_mode_test drm_dp_mst_calc_pbn_mode_cases
> >  	},
> >  	{
> >  		.clock = 332880,
> > -		.bpp = 24,
> > +		.bpp = 24 << 4,
> >  		.dsc = true,
> > -		.expected = 50
> > +		.expected = 1191
> >  	},
> >  	{
> >  		.clock = 324540,
> > -		.bpp = 24,
> > +		.bpp = 24 << 4,
> >  		.dsc = true,
> > -		.expected = 49
> > +		.expected = 1161
> >  	},
> >  };
> >  
> > -- 
> > 2.37.2
> 
> -- 
> Ville Syrjälä
> Intel