[PATCH v4 1/5] drm/dp-mst: Add PBN calculation for DSC modes

Lyude Paul lyude at redhat.com
Thu Aug 22 21:42:55 UTC 2019


Agh, sorry to go back on this - I'm noticing a lot more changes we should have
now that I've been able apply this locally to my tree. See below:

On Thu, 2019-08-22 at 09:57 -0400, David Francis wrote:
> With DSC, bpp can be a multiple of 1/16, so
> drm_dp_calc_pbn_mode is insufficient.
> 
> Add drm_dp_calc_pbn_mode_dsc, a function which is
> the same as drm_dp_calc_pbn_mode, but the bpp is
> in units of 1/16.
> 
> Reviewed-by: Lyude Paul <lyude at redhat.com>
> Signed-off-by: David Francis <David.Francis at amd.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 43 +++++++++++++++++++++++++++
>  include/drm/drm_dp_mst_helper.h       |  2 +-
>  2 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 82add736e17d..8e2e731c35c5 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -3591,6 +3591,49 @@ static int test_calc_pbn_mode(void)
>  	return 0;
>  }
>  
> +/**
> + * drm_dp_calc_pbn_mode_dsc() - Calculate the PBN for a mode with DSC
> enabled.
> + * @clock: dot clock for the mode
> + * @dsc_bpp: dsc bits per pixel x16 (e.g. dsc_bpp = 136 is 8.5 bpp)
> + *
> + * This uses the formula in the spec to calculate the PBN value for a mode,
> + * given that the mode is using DSC
> + * Returns:
> + * PBN required for this mode
> + */
> +int drm_dp_calc_pbn_mode_dsc(int clock, int dsc_bpp)
> +{
> +	u64 kbps;
> +	s64 peak_kbps;
> +	u32 numerator;
> +	u32 denominator;
> +
> +	kbps = clock * dsc_bpp;
> +
> +	/*
> +	 * margin 5300ppm + 300ppm ~ 0.6% as per spec, factor is 1.006
> +	 * The unit of 54/64Mbytes/sec is an arbitrary unit chosen based on
> +	 * common multiplier to render an integer PBN for all link rate/lane
> +	 * counts combinations
> +	 * calculate
> +	 * peak_kbps *= (1/16) bppx16 to bpp
> +	 * peak_kbps *= (1006/1000)
> +	 * peak_kbps *= (64/54)
> +	 * peak_kbps *= 8    convert to bytes
> +	 *
> +	 * Divide numerator and denominator by 16 to avoid overflow
> +	 */
> +
> +	numerator = 64 * 1006 / 16;
> +	denominator = 54 * 8 * 1000 * 1000;
> +
> +	kbps *= numerator;
> +	peak_kbps = drm_fixp_from_fraction(kbps, denominator);
> +
> +	return drm_fixp2int_ceil(peak_kbps);
> +}
> +EXPORT_SYMBOL(drm_dp_calc_pbn_mode_dsc);

I didn't notice until now that the only difference between the two functions
is

numerator = 64 * 1006 / 16;

So it doesn't seem like it's worth having two seperate functions for this, so
let's just get rid of drm_dp_calc_pbn_mode_dsc() and combine it with
drm_dp_calc_pbn_mode() by adding a bool parameter to specify whether or not
we're calculating for dsc

> +
>  /* we want to kick the TX after we've ack the up/down IRQs. */
>  static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr)
>  {
> diff --git a/include/drm/drm_dp_mst_helper.h
> b/include/drm/drm_dp_mst_helper.h
> index 2ba6253ea6d3..ddb518f2157a 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -611,7 +611,7 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector
> *connector, struct drm_dp_
>  
>  
>  int drm_dp_calc_pbn_mode(int clock, int bpp);
> -
> +int drm_dp_calc_pbn_mode_dsc(int clock, int dsc_bpp);
>  
>  bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
>  			      struct drm_dp_mst_port *port, int pbn, int
> slots);
-- 
Cheers,
	Lyude Paul



More information about the dri-devel mailing list