From: Jani Nikula <jani.nikula@linux.intel.com>
To: Imre Deak <imre.deak@intel.com>, intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 02/17] drm/i915/dp: Track the pipe and link bpp limits separately
Date: Thu, 17 Aug 2023 19:27:24 +0300 [thread overview]
Message-ID: <87fs4h8wib.fsf@intel.com> (raw)
In-Reply-To: <20230817161456.3857111-3-imre.deak@intel.com>
On Thu, 17 Aug 2023, Imre Deak <imre.deak@intel.com> wrote:
> A follow-up patch will need to limit the output link bpp both in the
> non-DSC and DSC configuration, so track the pipe and link bpp limits
> separately in the link_config_limits struct.
>
> Use .4 fixed point format for link bpp matching the 1/16 bpp granularity
> in DSC mode and for now keep this limit matching the pipe bpp limit.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++------
> drivers/gpu/drm/i915/display/intel_dp.h | 9 ++++++++-
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 17 +++++++++++------
> 3 files changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 89de444cfc4da..f4952fcfb16e9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1419,7 +1419,7 @@ intel_dp_adjust_compliance_config(struct intel_dp *intel_dp,
> if (intel_dp->compliance.test_data.bpc != 0) {
> int bpp = 3 * intel_dp->compliance.test_data.bpc;
>
> - limits->min_bpp = limits->max_bpp = bpp;
> + limits->pipe.min_bpp = limits->pipe.max_bpp = bpp;
> pipe_config->dither_force_disable = bpp == 6 * 3;
>
> drm_dbg_kms(&i915->drm, "Setting pipe_bpp to %d\n", bpp);
> @@ -1481,7 +1481,9 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
> int bpp, i, lane_count, clock = intel_dp_mode_clock(pipe_config, conn_state);
> int mode_rate, link_rate, link_avail;
>
> - for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
> + for (bpp = limits->link.max_bpp >> 4;
> + bpp >= limits->link.min_bpp >> 4;
I think I'd like to see some helpers for the >> 4 and << 4, to make this
self-documenting code.
to_bpp_int(), to_bpp_x16(), or something along those lines maybe.
With proper variable/member naming, you'd get:
bpp = to_bpp_int(bpp_x16);
bpp_x16 = to_bpp_x16(bpp);
And it would be obvious what's going on.
> + bpp -= 2 * 3) {
> int output_bpp = intel_dp_output_bpp(pipe_config->output_format, bpp);
>
> mode_rate = intel_dp_link_required(clock, output_bpp);
> @@ -1812,9 +1814,9 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp,
> limits->min_lane_count = 1;
> limits->max_lane_count = intel_dp_max_lane_count(intel_dp);
>
> - limits->min_bpp = intel_dp_min_bpp(crtc_state->output_format);
> - limits->max_bpp = intel_dp_max_bpp(intel_dp, crtc_state,
> - respect_downstream_limits);
> + limits->pipe.min_bpp = intel_dp_min_bpp(crtc_state->output_format);
> + limits->pipe.max_bpp = intel_dp_max_bpp(intel_dp, crtc_state,
> + respect_downstream_limits);
>
> if (intel_dp->use_max_params) {
> /*
> @@ -1831,10 +1833,13 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp,
>
> intel_dp_adjust_compliance_config(intel_dp, crtc_state, limits);
>
> + limits->link.min_bpp = limits->pipe.min_bpp << 4;
> + limits->link.max_bpp = limits->pipe.max_bpp << 4;
> +
> drm_dbg_kms(&i915->drm, "DP link computation with max lane count %i "
> "max rate %d max bpp %d pixel clock %iKHz\n",
> limits->max_lane_count, limits->max_rate,
> - limits->max_bpp, adjusted_mode->crtc_clock);
> + limits->link.max_bpp >> 4, adjusted_mode->crtc_clock);
> }
>
> static int
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 22099de3ca458..a1789419c0d19 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -26,7 +26,14 @@ struct intel_encoder;
> struct link_config_limits {
> int min_rate, max_rate;
> int min_lane_count, max_lane_count;
> - int min_bpp, max_bpp;
> + struct {
> + /* Uncompressed DSC input or link output bpp in 1 bpp units */
> + int min_bpp, max_bpp;
> + } pipe;
> + struct {
> + /* Compressed or uncompressed link output bpp in 1/16 bpp units */
> + int min_bpp, max_bpp;
The 1/16 bpp units is a source of confusion, and I think we should start
denoting them in naming.
min_bpp_x16, max_bpp_x16
> + } link;
> };
>
> void intel_edp_fixup_vbt_bpp(struct intel_encoder *encoder, int pipe_bpp);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 998d8a186cc6f..1809643538d08 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -156,8 +156,10 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
> &crtc_state->hw.adjusted_mode;
> int slots = -EINVAL;
>
> - slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state, limits->max_bpp,
> - limits->min_bpp, limits,
> + slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state,
> + limits->link.max_bpp >> 4,
> + limits->link.min_bpp >> 4,
> + limits,
> conn_state, 2 * 3, false);
>
> if (slots < 0)
> @@ -200,8 +202,8 @@ static int intel_dp_dsc_mst_compute_link_config(struct intel_encoder *encoder,
> else
> dsc_max_bpc = min_t(u8, 10, conn_state->max_requested_bpc);
>
> - max_bpp = min_t(u8, dsc_max_bpc * 3, limits->max_bpp);
> - min_bpp = limits->min_bpp;
> + max_bpp = min_t(u8, dsc_max_bpc * 3, limits->pipe.max_bpp);
> + min_bpp = limits->pipe.min_bpp;
>
> num_bpc = drm_dp_dsc_sink_supported_input_bpcs(intel_dp->dsc_dpcd,
> dsc_bpc);
> @@ -318,7 +320,7 @@ intel_dp_mst_compute_config_limits(struct intel_dp *intel_dp,
> limits->min_lane_count = limits->max_lane_count =
> intel_dp_max_lane_count(intel_dp);
>
> - limits->min_bpp = intel_dp_min_bpp(crtc_state->output_format);
> + limits->pipe.min_bpp = intel_dp_min_bpp(crtc_state->output_format);
> /*
> * FIXME: If all the streams can't fit into the link with
> * their current pipe_bpp we should reduce pipe_bpp across
> @@ -327,9 +329,12 @@ intel_dp_mst_compute_config_limits(struct intel_dp *intel_dp,
> * MST streams previously. This hack should be removed once
> * we have the proper retry logic in place.
> */
> - limits->max_bpp = min(crtc_state->pipe_bpp, 24);
> + limits->pipe.max_bpp = min(crtc_state->pipe_bpp, 24);
>
> intel_dp_adjust_compliance_config(intel_dp, crtc_state, limits);
> +
> + limits->link.min_bpp = limits->pipe.min_bpp << 4;
> + limits->link.max_bpp = limits->pipe.max_bpp << 4;
> }
>
> static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
--
Jani Nikula, Intel Open Source Graphics Center
next prev parent reply other threads:[~2023-08-17 16:28 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-17 16:14 [Intel-gfx] [PATCH 00/17] drm/i915: Improve BW management on shared display links Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 01/17] drm/i915/dp: Factor out helpers to compute the link limits Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 02/17] drm/i915/dp: Track the pipe and link bpp limits separately Imre Deak
2023-08-17 16:27 ` Jani Nikula [this message]
2023-08-18 8:24 ` Kandpal, Suraj
2023-08-18 14:06 ` Imre Deak
2023-08-18 13:26 ` Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 03/17] drm/i915/dp: Skip computing a non-DSC link config if DSC is needed Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 04/17] drm/i915/dp: Update the link bpp limits for DSC mode Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 05/17] drm/i915/dp: Limit the output link bpp in " Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 06/17] drm/i915: Add helper to modeset a set of pipes Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 07/17] drm/i915: Factor out a helper to check/compute all the CRTC states Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 08/17] drm/i915/fdi: Improve FDI BW sharing between pipe B and C Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 09/17] drm/dp_mst: Fix fractional bpp scaling in drm_dp_calc_pbn_mode() Imre Deak
2023-08-17 16:14 ` Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 10/17] drm/dp_mst: Add a way to calculate PBN values with FEC overhead Imre Deak
2023-08-17 16:14 ` Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 11/17] drm/dp_mst: Add helper to determine if an MST port is downstream of another port Imre Deak
2023-08-17 16:14 ` Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 12/17] drm/dp_mst: Factor out a helper to check the atomic state of a topology manager Imre Deak
2023-08-17 16:14 ` Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 13/17] drm/dp_mst: Swap the order of checking root vs. non-root port BW limitations Imre Deak
2023-08-17 16:14 ` Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 14/17] drm/i915/dp_mst: Fix PBN calculation with FEC overhead Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 15/17] drm/i915/dp_mst: Add atomic state for all streams on pre-tgl platforms Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 16/17] drm/i915/dp_mst: Improve BW sharing between MST streams Imre Deak
2023-08-17 16:14 ` [Intel-gfx] [PATCH 17/17] drm/i915/dp_mst: Check BW limitations only when all streams are computed Imre Deak
2023-08-18 1:33 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Improve BW management on shared display links Patchwork
2023-08-18 1:33 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-08-18 1:49 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-08-19 2:20 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
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=87fs4h8wib.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=imre.deak@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.