From: Jani Nikula <jani.nikula@intel.com>
To: imre.deak@intel.com, Lyude Paul <lyude@redhat.com>,
Wayne Lin <Wayne.Lin@amd.com>,
Alex Deucher <alexander.deucher@amd.com>,
Harry Wentland <harry.wentland@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
intel-gfx@lists.freedesktop.org,
Maxime Ripard <mripard@kernel.org>,
Thomas Zimmermann <tzimmermann@suse.de>,
Dave Airlie <airlied@redhat.com>,
Mikita Lipski <mikita.lipski@amd.com>,
David Francis <David.Francis@amd.com>
Subject: Re: [Intel-gfx] [PATCH v4 02/30] drm/dp_mst: Fix fractional DSC bpp handling
Date: Wed, 01 Nov 2023 14:59:50 +0200 [thread overview]
Message-ID: <87lebh4ojd.fsf@intel.com> (raw)
In-Reply-To: <ZUFa8NvCeWs+XT3X@ideak-desk.fi.intel.com>
On Tue, 31 Oct 2023, Imre Deak <imre.deak@intel.com> wrote:
> On Mon, Oct 30, 2023 at 05:58:15PM +0200, Imre Deak wrote:
> Hi Lyude, AMD folks et al,
>
> could you ack patches 2-9 in this patchset if they are ok and it's ok to
> merge them via the i915 tree?
Need acks from drm-misc maintainers too!
Cc: Maxime, Thomas, Maarten
>
> Thanks,
> Imre
>
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> The current code does '(bpp << 4) / 16' in the MST PBN
>> calculation, but that is just the same as 'bpp' so the
>> DSC codepath achieves absolutely nothing. Fix it up so that
>> the fractional part of the bpp value is actually used instead
>> of truncated away. 64*1006 has enough zero lsbs that we can
>> just shift that down in the dividend and thus still manage
>> to stick to a 32bit divisor.
>>
>> And while touching this, let's just make the whole thing more
>> straightforward by making the passed in bpp value .4 binary
>> fixed point always, instead of having to pass in different
>> things based on whether DSC is enabled or not.
>>
>> v2:
>> - Fix DSC kunit test cases.
>>
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Lyude Paul <lyude@redhat.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Cc: David Francis <David.Francis@amd.com>
>> Cc: Mikita Lipski <mikita.lipski@amd.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Fixes: dc48529fb14e ("drm/dp_mst: Add PBN calculation for DSC modes")
>> Reviewed-by: Lyude Paul <lyude@redhat.com> (v1)
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> [Imre: Fix kunit test cases]
>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>> ---
>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>> .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +-
>> drivers/gpu/drm/display/drm_dp_mst_topology.c | 20 +++++--------------
>> drivers/gpu/drm/i915/display/intel_dp_mst.c | 5 ++---
>> drivers/gpu/drm/nouveau/dispnv50/disp.c | 3 +--
>> .../gpu/drm/tests/drm_dp_mst_helper_test.c | 6 +++---
>> include/drm/display/drm_dp_mst_helper.h | 2 +-
>> 7 files changed, 14 insertions(+), 26 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 9a712791f309f..ada3773869ff0 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -6918,7 +6918,7 @@ static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
>> max_bpc);
>> 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, false);
>> + dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp << 4);
>> }
>>
>> dm_new_connector_state->vcpi_slots =
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> index d3b13d362edac..9a58e1a4c5f49 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> @@ -1642,7 +1642,7 @@ enum dc_status dm_dp_mst_is_port_support_mode(
>> } else {
>> /* check if mode could be supported within full_pbn */
>> bpp = convert_dc_color_depth_into_bpc(stream->timing.display_color_depth) * 3;
>> - pbn = drm_dp_calc_pbn_mode(stream->timing.pix_clk_100hz / 10, bpp, false);
>> + pbn = drm_dp_calc_pbn_mode(stream->timing.pix_clk_100hz / 10, bpp << 4);
>>
>> if (pbn > aconnector->mst_output_port->full_pbn)
>> return DC_FAIL_BANDWIDTH_VALIDATE;
>> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> index 0e0d0e76de065..772b00ebd57bd 100644
>> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> @@ -4718,13 +4718,12 @@ 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
>> + * @clock: dot clock
>> + * @bpp: bpp as .4 binary fixed point
>> *
>> * 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, bool dsc)
>> +int drm_dp_calc_pbn_mode(int clock, int bpp)
>> {
>> /*
>> * margin 5300ppm + 300ppm ~ 0.6% as per spec, factor is 1.006
>> @@ -4735,18 +4734,9 @@ int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc)
>> * 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);
>> + return DIV_ROUND_UP_ULL(mul_u32_u32(clock * bpp, 64 * 1006 >> 4),
>> + 1000 * 8 * 54 * 1000);
>> }
>> EXPORT_SYMBOL(drm_dp_calc_pbn_mode);
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> index 851b312bd8449..5bf45a2a85b0e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> @@ -106,8 +106,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
>> continue;
>>
>> crtc_state->pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock,
>> - dsc ? bpp << 4 : bpp,
>> - dsc);
>> + bpp << 4);
>>
>> slots = drm_dp_atomic_find_time_slots(state, &intel_dp->mst_mgr,
>> connector->port,
>> @@ -975,7 +974,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>> return ret;
>>
>> if (mode_rate > max_rate || mode->clock > max_dotclk ||
>> - drm_dp_calc_pbn_mode(mode->clock, min_bpp, false) > port->full_pbn) {
>> + drm_dp_calc_pbn_mode(mode->clock, min_bpp << 4) > port->full_pbn) {
>> *status = MODE_CLOCK_HIGH;
>> return 0;
>> }
>> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
>> index d2be40337b92e..153717e1df1a2 100644
>> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
>> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
>> @@ -982,8 +982,7 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
>> const int clock = crtc_state->adjusted_mode.clock;
>>
>> asyh->or.bpc = connector->display_info.bpc;
>> - asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, asyh->or.bpc * 3,
>> - false);
>> + asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, asyh->or.bpc * 3 << 4);
>> }
>>
>> mst_state = drm_atomic_get_mst_topology_state(state, &mstm->mgr);
>> 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..e3c818dfc0e6d 100644
>> --- a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
>> +++ b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
>> @@ -42,13 +42,13 @@ static const struct drm_dp_mst_calc_pbn_mode_test drm_dp_mst_calc_pbn_mode_cases
>> .clock = 332880,
>> .bpp = 24,
>> .dsc = true,
>> - .expected = 50
>> + .expected = 1191
>> },
>> {
>> .clock = 324540,
>> .bpp = 24,
>> .dsc = true,
>> - .expected = 49
>> + .expected = 1161
>> },
>> };
>>
>> @@ -56,7 +56,7 @@ static void drm_test_dp_mst_calc_pbn_mode(struct kunit *test)
>> {
>> const struct drm_dp_mst_calc_pbn_mode_test *params = test->param_value;
>>
>> - KUNIT_EXPECT_EQ(test, drm_dp_calc_pbn_mode(params->clock, params->bpp, params->dsc),
>> + KUNIT_EXPECT_EQ(test, drm_dp_calc_pbn_mode(params->clock, params->bpp << 4),
>> params->expected);
>> }
>>
>> diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
>> index 4429d3b1745b6..655862b3d2a49 100644
>> --- a/include/drm/display/drm_dp_mst_helper.h
>> +++ b/include/drm/display/drm_dp_mst_helper.h
>> @@ -842,7 +842,7 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector,
>> int drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr,
>> int link_rate, int link_lane_count);
>>
>> -int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc);
>> +int drm_dp_calc_pbn_mode(int clock, int bpp);
>>
>> void drm_dp_mst_update_slots(struct drm_dp_mst_topology_state *mst_state, uint8_t link_encoding_cap);
>>
>> --
>> 2.39.2
>>
--
Jani Nikula, Intel
next prev parent reply other threads:[~2023-11-01 13:00 UTC|newest]
Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-30 15:58 [Intel-gfx] [PATCH v4 00/30] drm/i915: Improve BW management on MST links Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 01/30] drm/i915/dp_mst: Fix race between connector registration and setup Imre Deak
2023-10-31 9:23 ` Lisovskiy, Stanislav
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 02/30] drm/dp_mst: Fix fractional DSC bpp handling Imre Deak
2023-10-31 19:52 ` Imre Deak
2023-11-01 12:59 ` Jani Nikula [this message]
2023-11-06 8:16 ` Maxime Ripard
2023-11-07 22:45 ` Lyude Paul
2023-11-08 14:40 ` Harry Wentland
2023-11-08 17:01 ` Deucher, Alexander
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 03/30] drm/dp_mst: Add helper to determine if an MST port is downstream of another port Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 04/30] drm/dp_mst: Factor out a helper to check the atomic state of a topology manager Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 05/30] drm/dp_mst: Swap the order of checking root vs. non-root port BW limitations Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 06/30] drm/dp_mst: Allow DSC in any Synaptics last branch device Imre Deak
2023-11-07 22:35 ` Lyude Paul
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 07/30] drm/dp: Add DP_HBLANK_EXPANSION_CAPABLE and DSC_PASSTHROUGH_EN DPCD flags Imre Deak
2023-11-07 22:35 ` Lyude Paul
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 08/30] drm/dp_mst: Add HBLANK expansion quirk for Synaptics MST hubs Imre Deak
2023-11-07 22:37 ` Lyude Paul
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 09/30] drm/dp: Add helpers to calculate the link BW overhead Imre Deak
2023-11-06 21:31 ` Ville Syrjälä
2023-11-06 22:28 ` Imre Deak
2023-11-07 16:24 ` Ville Syrjälä
2023-11-07 0:14 ` [Intel-gfx] [PATCH v5 " Imre Deak
2023-11-07 22:45 ` Lyude Paul
2023-11-07 22:42 ` [Intel-gfx] [PATCH v4 " Lyude Paul
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 10/30] drm/i915/dp_mst: Enable FEC early once it's known DSC is needed Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 11/30] drm/i915/dp: Specify the FEC overhead as an increment vs. a remainder Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 12/30] drm/i915/dp: Pass actual BW overhead to m_n calculation Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 13/30] drm/i915/dp_mst: Account for FEC and DSC overhead during BW allocation Imre Deak
2023-11-06 20:39 ` Ville Syrjälä
2023-11-06 21:02 ` Imre Deak
2023-11-06 21:15 ` Ville Syrjälä
2023-11-06 21:29 ` Imre Deak
2023-11-06 20:49 ` Ville Syrjälä
2023-11-06 21:54 ` Imre Deak
2023-11-07 0:14 ` [Intel-gfx] [PATCH v5 " Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 14/30] drm/i915/dp_mst: Add atomic state for all streams on pre-tgl platforms Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 15/30] drm/i915/dp_mst: Program the DSC PPS SDP for each stream Imre Deak
2023-11-07 0:15 ` [Intel-gfx] [PATCH v5 " Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 16/30] drm/i915/dp: Make sure the DSC PPS SDP is disabled whenever DSC is disabled Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 17/30] drm/i915/dp_mst: Add missing DSC compression disabling Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 18/30] drm/i915/dp: Rename intel_ddi_disable_fec_state() to intel_ddi_disable_fec() Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 19/30] drm/i915/dp: Wait for FEC detected status in the sink Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 20/30] drm/i915/dp: Disable FEC ready flag " Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 21/30] drm/i915/dp_mst: Handle the Synaptics HBlank expansion quirk Imre Deak
2023-11-07 0:15 ` [Intel-gfx] [PATCH v5 " Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 22/30] drm/i915/dp_mst: Enable decompression in the sink from the MST encoder hooks Imre Deak
2023-11-07 0:15 ` [Intel-gfx] [PATCH v5 " Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 23/30] drm/i915/dp: Enable DSC via the connector decompression AUX Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 24/30] drm/i915/dp_mst: Enable DSC passthrough Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 25/30] drm/i915/dp_mst: Enable MST DSC decompression for all streams Imre Deak
2023-10-31 8:47 ` Lisovskiy, Stanislav
2023-11-07 0:15 ` [Intel-gfx] [PATCH v5 " Imre Deak
2023-11-08 8:09 ` Lisovskiy, Stanislav
2024-02-02 17:48 ` [v5, " Drew Davenport
2024-02-05 13:38 ` Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 26/30] drm/i915: Factor out function to clear pipe update flags Imre Deak
2023-11-01 10:17 ` Ville Syrjälä
2023-11-01 11:38 ` Imre Deak
2023-11-07 0:15 ` [Intel-gfx] [PATCH v5 " Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 27/30] drm/i915/dp_mst: Force modeset CRTC if DSC toggling requires it Imre Deak
2023-11-07 0:15 ` [Intel-gfx] [PATCH v5 " Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 28/30] drm/i915/dp_mst: Improve BW sharing between MST streams Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 29/30] drm/i915/dp_mst: Check BW limitations only after all streams are computed Imre Deak
2023-10-30 15:58 ` [Intel-gfx] [PATCH v4 30/30] drm/i915: Query compressed bpp properly using correct DPCD and DP Spec info Imre Deak
2023-10-30 23:39 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Improve BW management on MST links (rev8) Patchwork
2023-10-30 23:39 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-10-30 23:57 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2023-10-31 14:16 ` Imre Deak
2023-11-01 4:32 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-11-02 11:44 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2023-11-03 22:43 ` [Intel-gfx] [PATCH v4 00/30] drm/i915: Improve BW management on MST links Lyude Paul
2023-11-07 1:28 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Improve BW management on MST links (rev16) Patchwork
2023-11-07 1:28 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-11-07 1:41 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-11-07 9:50 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2023-11-08 15:59 ` Imre Deak
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=87lebh4ojd.fsf@intel.com \
--to=jani.nikula@intel.com \
--cc=David.Francis@amd.com \
--cc=Wayne.Lin@amd.com \
--cc=airlied@redhat.com \
--cc=alexander.deucher@amd.com \
--cc=daniel.vetter@ffwll.ch \
--cc=harry.wentland@amd.com \
--cc=imre.deak@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=lyude@redhat.com \
--cc=mikita.lipski@amd.com \
--cc=mripard@kernel.org \
--cc=tzimmermann@suse.de \
/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