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 <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Imre Deak <[email protected]>
> > ---
> > 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
