From: Lyude Paul <lyude@redhat.com>
To: mikita.lipski@amd.com, amd-gfx@lists.freedesktop.org
Cc: David Francis <David.Francis@amd.com>, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v9 01/18] drm/dp_mst: Add PBN calculation for DSC modes
Date: Fri, 20 Dec 2019 16:35:45 -0500 [thread overview]
Message-ID: <78e9ebfb314efc592bff859daf836e368d2c5eef.camel@redhat.com> (raw)
In-Reply-To: <20191213200854.31545-2-mikita.lipski@amd.com>
Actually, one comment on this that should be very simple to add
On Fri, 2019-12-13 at 15:08 -0500, mikita.lipski@amd.com wrote:
> From: David Francis <David.Francis@amd.com>
>
> With DSC, bpp can be fractional in multiples of 1/16.
>
> Change drm_dp_calc_pbn_mode to reflect this, adding a new
> parameter bool dsc. When this parameter is true, treat the
> bpp parameter as having units not of bits per pixel, but
> 1/16 of a bit per pixel
>
> v2: Don't add separate function for this
> v3: In the equation divide bpp by 16 as it is expected
> not to leave any remainder
>
> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> Reviewed-by: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: David Francis <David.Francis@amd.com>
> Signed-off-by: Mikita Lipski <mikita.lipski@amd.com>
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
> drivers/gpu/drm/drm_dp_mst_topology.c | 12 +++++++++++-
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 ++-
> drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
> drivers/gpu/drm/radeon/radeon_dp_mst.c | 2 +-
> drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c | 10 ++++++----
> include/drm/drm_dp_mst_helper.h | 3 +--
> 7 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 455c51c38720..9fc03fc1017d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4967,7 +4967,7 @@ static int dm_encoder_helper_atomic_check(struct
> drm_encoder *encoder,
> is_y420);
> bpp = convert_dc_color_depth_into_bpc(color_depth) * 3;
> clock = adjusted_mode->clock;
> - dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock,
> bpp);
> + dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp,
> false);
> }
> dm_new_connector_state->vcpi_slots =
> drm_dp_atomic_find_vcpi_slots(state,
> mst
> _mgr,
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index ae5809a1f19a..363e7e58e7e7 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4342,10 +4342,11 @@ EXPORT_SYMBOL(drm_dp_check_act_status);
> * drm_dp_calc_pbn_mode() - Calculate the PBN for a mode.
> * @clock: dot clock for the mode
> * @bpp: bpp for the mode.
> + * @dsc: DSC mode. If true, bpp has units of 1/16 of a bit per pixel
> *
> * This uses the formula in the spec to calculate the PBN value for a mode.
> */
> -int drm_dp_calc_pbn_mode(int clock, int bpp)
> +int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc)
> {
> /*
> * margin 5300ppm + 300ppm ~ 0.6% as per spec, factor is 1.006
> @@ -4356,7 +4357,16 @@ int drm_dp_calc_pbn_mode(int clock, int bpp)
> * peak_kbps *= (1006/1000)
> * peak_kbps *= (64/54)
> * peak_kbps *= 8 convert to bytes
> + *
> + * If the bpp is in units of 1/16, further divide by 16. Put this
> + * factor in the numerator rather than the denominator to avoid
> + * integer overflow
> */
> +
> + 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),
> 8 * 54 * 1000 * 1000);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 03d1cba0b696..92be17711287 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -61,7 +61,8 @@ static int intel_dp_mst_compute_link_config(struct
> intel_encoder *encoder,
> crtc_state->pipe_bpp = bpp;
>
> crtc_state->pbn = drm_dp_calc_pbn_mode(adjusted_mode-
> >crtc_clock,
> - crtc_state->pipe_bpp);
> + crtc_state->pipe_bpp,
> + false);
>
> slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp-
> >mst_mgr,
> port, crtc_state->pbn);
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 549486f1d937..1c9e23d5a6fd 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -782,7 +782,7 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
> const int bpp = connector->display_info.bpc * 3;
> const int clock = crtc_state->adjusted_mode.clock;
>
> - asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, bpp);
> + asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, bpp,
> false);
> }
>
> slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr,
> diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> index ee28f5b3785e..28eef9282874 100644
> --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> @@ -518,7 +518,7 @@ static bool radeon_mst_mode_fixup(struct drm_encoder
> *encoder,
>
> mst_enc = radeon_encoder->enc_priv;
>
> - mst_enc->pbn = drm_dp_calc_pbn_mode(adjusted_mode->clock, bpp);
> + mst_enc->pbn = drm_dp_calc_pbn_mode(adjusted_mode->clock, bpp, false);
>
> mst_enc->primary->active_device = mst_enc->primary->devices & mst_enc-
> >connector->devices;
> DRM_DEBUG_KMS("setting active device to %08x from %08x %08x for
> encoder %d\n",
> diff --git a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> index af2b2de65316..73fc1c485283 100644
> --- a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> +++ b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> @@ -18,15 +18,17 @@ int igt_dp_mst_calc_pbn_mode(void *ignored)
> int rate;
> int bpp;
> int expected;
> + bool dsc;
> } test_params[] = {
> - { 154000, 30, 689 },
> - { 234000, 30, 1047 },
> - { 297000, 24, 1063 },
> + { 154000, 30, 689, false },
> + { 234000, 30, 1047, false },
> + { 297000, 24, 1063, false },
Mind adding one or two test_params that actually use dsc here?
> };
>
> for (i = 0; i < ARRAY_SIZE(test_params); i++) {
> pbn = drm_dp_calc_pbn_mode(test_params[i].rate,
> - test_params[i].bpp);
> + test_params[i].bpp,
> + test_params[i].dsc);
> FAIL(pbn != test_params[i].expected,
> "Expected PBN %d for clock %d bpp %d, got %d\n",
> test_params[i].expected, test_params[i].rate,
> diff --git a/include/drm/drm_dp_mst_helper.h
> b/include/drm/drm_dp_mst_helper.h
> index d5fc90b30487..68656913cfe5 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -719,8 +719,7 @@ bool drm_dp_mst_port_has_audio(struct
> drm_dp_mst_topology_mgr *mgr,
> struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct
> drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
>
>
> -int drm_dp_calc_pbn_mode(int clock, int bpp);
> -
> +int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc);
>
> 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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2019-12-20 21:35 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-13 20:08 [PATCH v9 00/18] DSC MST support for DRM and AMDGPU mikita.lipski
2019-12-13 20:08 ` [PATCH v9 01/18] drm/dp_mst: Add PBN calculation for DSC modes mikita.lipski
2019-12-20 21:35 ` Lyude Paul [this message]
2019-12-13 20:08 ` [PATCH v9 02/18] drm/dp_mst: Parse FEC capability on MST ports mikita.lipski
2019-12-13 20:08 ` [PATCH v9 03/18] drm/dp_mst: Add MST support to DP DPCD R/W functions mikita.lipski
2019-12-13 20:08 ` [PATCH v9 04/18] drm/dp_mst: Fill branch->num_ports mikita.lipski
2019-12-13 20:08 ` [PATCH v9 05/18] drm/dp_mst: Add helpers for MST DSC and virtual DPCD aux mikita.lipski
2019-12-13 20:08 ` [PATCH v9 06/18] drm/dp_mst: Add new quirk for Synaptics MST hubs mikita.lipski
2019-12-13 20:08 ` [PATCH v9 07/18] drm/amd/display: Initialize DSC PPS variables to 0 mikita.lipski
2019-12-13 20:08 ` [PATCH v9 08/18] drm/amd/display: Validate DSC caps on MST endpoints mikita.lipski
2019-12-13 20:08 ` [PATCH v9 09/18] drm/amd/display: Write DSC enable to MST DPCD mikita.lipski
2019-12-13 20:08 ` [PATCH v9 10/18] drm/dp_mst: Manually overwrite PBN divider for calculating timeslots mikita.lipski
2019-12-20 21:45 ` Lyude Paul
2019-12-13 20:08 ` [PATCH v9 11/18] drm/dp_mst: Add DSC enablement helpers to DRM mikita.lipski
2019-12-13 20:08 ` [PATCH v9 12/18] drm/dp_mst: Add branch bandwidth validation to MST atomic check mikita.lipski
2020-01-17 15:09 ` Sean Paul
2020-01-17 15:26 ` Mikita Lipski
2020-01-17 15:39 ` Sean Paul
2020-01-17 20:26 ` Lyude Paul
2019-12-13 20:08 ` [PATCH v9 13/18] drm/dp_mst: Rename drm_dp_mst_atomic_check_topology_state mikita.lipski
2019-12-13 20:08 ` [PATCH v9 14/18] drm/amd/display: Add PBN per slot calculation for DSC mikita.lipski
2019-12-20 21:44 ` Lyude Paul
2019-12-20 22:50 ` Leo
2019-12-13 20:08 ` [PATCH v9 15/18] drm/amd/display: MST DSC compute fair share mikita.lipski
2019-12-20 21:44 ` Lyude Paul
2019-12-13 20:08 ` [PATCH v9 16/18] drm/amd/display: Recalculate VCPI slots for new DSC connectors mikita.lipski
2019-12-20 21:41 ` Lyude Paul
2019-12-23 19:05 ` Mikita Lipski
2019-12-13 20:08 ` [PATCH v9 17/18] drm/dp_mst: Add helper to trigger modeset on affected DSC MST CRTCs mikita.lipski
2019-12-13 20:08 ` [PATCH v9 18/18] drm/amd/display: Trigger modesets on MST DSC connectors mikita.lipski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=78e9ebfb314efc592bff859daf836e368d2c5eef.camel@redhat.com \
--to=lyude@redhat.com \
--cc=David.Francis@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=mikita.lipski@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).