From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Imre Deak <imre.deak@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 04/22] drm/i915/dp: Update the link bpp limits for DSC mode
Date: Mon, 4 Sep 2023 06:48:25 +0300 [thread overview]
Message-ID: <ZPVTiYWRnRRyIViQ@intel.com> (raw)
In-Reply-To: <20230824080517.693621-5-imre.deak@intel.com>
On Thu, Aug 24, 2023 at 11:04:59AM +0300, Imre Deak wrote:
> In non-DSC mode the link bpp can be set in 2*3 bpp steps in the pipe bpp
> range, while in DSC mode it can be set in 1/16 bpp steps to any value
> up to the maximum pipe bpp. Update the limits accordingly in both modes
> to prepare for a follow-up patch which may need to reduce the max link
> bpp value and starts to check the link bpp limits in DSC mode as well.
>
> While at it add more detail to the link limit debug print and print it
> also for DSC mode.
>
> v2:
> - Add to_bpp_frac_dec() instead of open coding it. (Jani)
>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> .../drm/i915/display/intel_display_types.h | 5 ++
> drivers/gpu/drm/i915/display/intel_dp.c | 89 +++++++++++++++----
> drivers/gpu/drm/i915/display/intel_dp.h | 6 ++
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 23 +++--
> 4 files changed, 101 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 5875eff5012ce..a0a404967b5d2 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -2113,6 +2113,11 @@ static inline int to_bpp_int(int bpp_x16)
> return bpp_x16 >> 4;
> }
>
> +static inline int to_bpp_frac_dec(int bpp_x16)
> +{
> + return (bpp_x16 & 0xf) * 625;
> +}
This gives me the impression that this would be somehow
generally useful, but I presume we only use it for the printk?
So maybe should just have some printk FMT+ARG macros for
this stuff?
> +
> static inline int to_bpp_x16(int bpp)
> {
> return bpp << 4;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index c580472c06b85..9ce861a7fd418 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2189,16 +2189,68 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> return 0;
> }
>
> -static void
> +/**
> + * intel_dp_compute_config_link_bpp_limits - compute output link bpp limits
> + * @intel_dp: intel DP
> + * @crtc_state: crtc state
> + * @dsc: DSC compression mode
> + * @limits: link configuration limits
> + *
> + * Calculates the output link min, max bpp values in @limits based on the
> + * pipe bpp range, @crtc_state and @dsc mode.
> + *
> + * Returns %true in case of success.
> + */
> +bool
> +intel_dp_compute_config_link_bpp_limits(struct intel_dp *intel_dp,
> + const struct intel_crtc_state *crtc_state,
> + bool dsc,
> + struct link_config_limits *limits)
> +{
> + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> + const struct drm_display_mode *adjusted_mode =
> + &crtc_state->hw.adjusted_mode;
> + const struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> + const struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> + int max_link_bpp_x16;
> +
> + max_link_bpp_x16 = to_bpp_x16(limits->pipe.max_bpp);
> +
> + if (!dsc) {
> + max_link_bpp_x16 = rounddown(max_link_bpp_x16, to_bpp_x16(2 * 3));
> +
> + if (max_link_bpp_x16 < to_bpp_x16(limits->pipe.min_bpp))
> + return false;
Quite a few to_bpp_x16()'s in there. Seems like it would a bit simpler
to just do that once at the end.
> +
> + limits->link.min_bpp_x16 = to_bpp_x16(limits->pipe.min_bpp);
> + } else {
> + limits->link.min_bpp_x16 = 0;
Why is that zero? Don't we now have some helpers to fill
this stuff correctly?
> + }
> +
> + limits->link.max_bpp_x16 = max_link_bpp_x16;
> +
> + drm_dbg_kms(&i915->drm,
> + "[ENCODER:%d:%s][CRTC:%d:%s] DP link limits: pixel clock %d kHz DSC %s max lanes %d max rate %d max pipe_bpp %d max link_bpp %d.%04d\n",
> + encoder->base.base.id, encoder->base.name,
> + crtc->base.base.id, crtc->base.name,
> + adjusted_mode->crtc_clock,
> + dsc ? "on" : "off",
> + limits->max_lane_count,
> + limits->max_rate,
> + limits->pipe.max_bpp,
> + to_bpp_int(limits->link.max_bpp_x16),
> + to_bpp_frac_dec(limits->link.max_bpp_x16));
> +
> + return true;
> +}
> +
> +static bool
> intel_dp_compute_config_limits(struct intel_dp *intel_dp,
> struct intel_crtc_state *crtc_state,
> bool respect_downstream_limits,
> + bool dsc,
> struct link_config_limits *limits)
> {
> - struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> - const struct drm_display_mode *adjusted_mode =
> - &crtc_state->hw.adjusted_mode;
> -
> limits->min_rate = intel_dp_common_rate(intel_dp, 0);
> limits->max_rate = intel_dp_max_link_rate(intel_dp);
>
> @@ -2224,13 +2276,10 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp,
>
> intel_dp_adjust_compliance_config(intel_dp, crtc_state, limits);
>
> - limits->link.min_bpp_x16 = to_bpp_x16(limits->pipe.min_bpp);
> - limits->link.max_bpp_x16 = to_bpp_x16(limits->pipe.max_bpp);
> -
> - 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,
> - to_bpp_int(limits->link.max_bpp_x16), adjusted_mode->crtc_clock);
> + return intel_dp_compute_config_link_bpp_limits(intel_dp,
> + crtc_state,
> + dsc,
> + limits);
> }
>
> static int
> @@ -2249,9 +2298,6 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
> bool dsc_needed;
> int ret = 0;
>
> - intel_dp_compute_config_limits(intel_dp, pipe_config,
> - respect_downstream_limits, &limits);
> -
> if (intel_dp_need_bigjoiner(intel_dp, adjusted_mode->crtc_hdisplay,
> adjusted_mode->crtc_clock))
> pipe_config->bigjoiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
> @@ -2263,7 +2309,11 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
> */
> joiner_needs_dsc = DISPLAY_VER(i915) < 13 && pipe_config->bigjoiner_pipes;
>
> - dsc_needed = joiner_needs_dsc || intel_dp->force_dsc_en;
> + dsc_needed = joiner_needs_dsc || intel_dp->force_dsc_en ||
> + !intel_dp_compute_config_limits(intel_dp, pipe_config,
> + respect_downstream_limits,
> + false,
> + &limits);
>
> if (!dsc_needed) {
> /*
> @@ -2280,6 +2330,13 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
> drm_dbg_kms(&i915->drm, "Try DSC (fallback=%s, joiner=%s, force=%s)\n",
> str_yes_no(ret), str_yes_no(joiner_needs_dsc),
> str_yes_no(intel_dp->force_dsc_en));
> +
> + if (!intel_dp_compute_config_limits(intel_dp, pipe_config,
> + respect_downstream_limits,
> + true,
> + &limits))
> + return -EINVAL;
> +
> ret = intel_dp_dsc_compute_config(intel_dp, pipe_config,
> conn_state, &limits, 64, true);
> if (ret < 0)
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index ebc7f4e60c777..0b8a4bbef8f7d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -153,4 +153,10 @@ void intel_dp_phy_test(struct intel_encoder *encoder);
> void intel_dp_wait_source_oui(struct intel_dp *intel_dp);
> int intel_dp_output_bpp(enum intel_output_format output_format, int bpp);
>
> +bool
> +intel_dp_compute_config_link_bpp_limits(struct intel_dp *intel_dp,
> + const struct intel_crtc_state *crtc_state,
> + bool dsc,
> + struct link_config_limits *limits);
> +
> #endif /* __INTEL_DP_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index c077b999ccb74..2b78a3a8966f3 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -308,9 +308,10 @@ static bool intel_dp_mst_has_audio(const struct drm_connector_state *conn_state)
> return intel_conn_state->force_audio == HDMI_AUDIO_ON;
> }
>
> -static void
> +static bool
> intel_dp_mst_compute_config_limits(struct intel_dp *intel_dp,
> struct intel_crtc_state *crtc_state,
> + bool dsc,
> struct link_config_limits *limits)
> {
> /*
> @@ -336,8 +337,10 @@ intel_dp_mst_compute_config_limits(struct intel_dp *intel_dp,
>
> intel_dp_adjust_compliance_config(intel_dp, crtc_state, limits);
>
> - limits->link.min_bpp_x16 = to_bpp_x16(limits->pipe.min_bpp);
> - limits->link.max_bpp_x16 = to_bpp_x16(limits->pipe.max_bpp);
> + return intel_dp_compute_config_link_bpp_limits(intel_dp,
> + crtc_state,
> + dsc,
> + limits);
> }
>
> static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
> @@ -364,9 +367,11 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
> intel_dp_mst_has_audio(conn_state) &&
> intel_audio_compute_config(encoder, pipe_config, conn_state);
>
> - intel_dp_mst_compute_config_limits(intel_dp, pipe_config, &limits);
> -
> - dsc_needed = intel_dp->force_dsc_en;
> + dsc_needed = intel_dp->force_dsc_en ||
> + !intel_dp_mst_compute_config_limits(intel_dp,
> + pipe_config,
> + false,
> + &limits);
>
> if (!dsc_needed) {
> ret = intel_dp_mst_compute_link_config(encoder, pipe_config,
> @@ -385,6 +390,12 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
> str_yes_no(ret),
> str_yes_no(intel_dp->force_dsc_en));
>
> + if (!intel_dp_mst_compute_config_limits(intel_dp,
> + pipe_config,
> + true,
> + &limits))
> + return -EINVAL;
> +
> /*
> * FIXME: As bpc is hardcoded to 8, as mentioned above,
> * WARN and ignore the debug flag force_dsc_bpc for now.
> --
> 2.37.2
--
Ville Syrjälä
Intel
next prev parent reply other threads:[~2023-09-04 3:48 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-24 8:04 [Intel-gfx] [PATCH v2 00/22] drm/i915: Improve BW management on shared display links Imre Deak
2023-08-24 8:04 ` [Intel-gfx] [PATCH v2 01/22] drm/i915/dp: Factor out helpers to compute the link limits Imre Deak
2023-09-04 3:19 ` Ville Syrjälä
2023-09-04 10:25 ` Imre Deak
2023-08-24 8:04 ` [Intel-gfx] [PATCH v2 02/22] drm/i915/dp: Track the pipe and link bpp limits separately Imre Deak
2023-09-14 9:33 ` Luca Coelho
2023-09-14 9:55 ` Imre Deak
2023-09-14 10:51 ` Luca Coelho
2023-09-14 11:08 ` Imre Deak
2023-09-14 12:08 ` Luca Coelho
2023-08-24 8:04 ` [Intel-gfx] [PATCH v2 03/22] drm/i915/dp: Skip computing a non-DSC link config if DSC is needed Imre Deak
2023-09-04 3:24 ` Ville Syrjälä
2023-08-24 8:04 ` [Intel-gfx] [PATCH v2 04/22] drm/i915/dp: Update the link bpp limits for DSC mode Imre Deak
2023-09-04 3:48 ` Ville Syrjälä [this message]
2023-09-04 11:08 ` Imre Deak
2023-09-05 5:25 ` Ville Syrjälä
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 05/22] drm/i915/dp: Limit the output link bpp in " Imre Deak
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 06/22] drm/i915: Add helper to modeset a set of pipes Imre Deak
2023-09-08 19:25 ` Ville Syrjälä
2023-09-08 20:08 ` Imre Deak
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 07/22] drm/i915: Factor out a helper to check/compute all the CRTC states Imre Deak
2023-09-08 19:31 ` Ville Syrjälä
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 08/22] drm/i915/fdi: Improve FDI BW sharing between pipe B and C Imre Deak
2023-09-11 18:59 ` Ville Syrjälä
2023-09-11 20:42 ` Imre Deak
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 09/22] drm/dp_mst: Fix fractional bpp scaling in drm_dp_calc_pbn_mode() Imre Deak
2023-08-30 21:27 ` Lyude Paul
2023-09-04 2:53 ` Ville Syrjälä
2023-09-04 10:22 ` Imre Deak
2023-09-06 10:45 ` Ville Syrjälä
2023-09-06 11:14 ` Imre Deak
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 10/22] drm/dp_mst: Add a way to calculate PBN values with FEC overhead Imre Deak
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 11/22] drm/dp_mst: Add helper to determine if an MST port is downstream of another port Imre Deak
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 12/22] drm/dp_mst: Factor out a helper to check the atomic state of a topology manager Imre Deak
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 13/22] drm/dp_mst: Swap the order of checking root vs. non-root port BW limitations Imre Deak
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 14/22] drm/i915/dp_mst: Fix PBN calculation with FEC overhead Imre Deak
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 15/22] drm/i915/dp_mst: Add atomic state for all streams on pre-tgl platforms Imre Deak
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 16/22] drm/i915/dp_mst: Program the DSC PPS SDP for each stream Imre Deak
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 17/22] drm/i915/dp: Make sure the DSC PPS SDP is disabled whenever DSC is disabled Imre Deak
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 18/22] drm/i915/dp_mst: Enable DSC decompression if any stream needs this Imre Deak
2023-08-24 9:31 ` Lisovskiy, Stanislav
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 19/22] drm/i915/dp_mst: Add missing DSC compression disabling Imre Deak
2023-08-24 9:44 ` Lisovskiy, Stanislav
2023-08-24 10:37 ` Imre Deak
2023-09-11 12:29 ` Lisovskiy, Stanislav
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 20/22] drm/i915/dp_mst: Allow DSC only for sink ports of the first branch device Imre Deak
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 21/22] drm/i915/dp_mst: Improve BW sharing between MST streams Imre Deak
2023-08-24 8:05 ` [Intel-gfx] [PATCH v2 22/22] drm/i915/dp_mst: Check BW limitations only after all streams are computed Imre Deak
2023-09-25 7:46 ` Lisovskiy, Stanislav
2023-08-24 9:54 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Improve BW management on shared display links (rev2) Patchwork
2023-08-24 9:54 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-08-24 10:07 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-08-24 17:54 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2023-09-04 7:14 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Improve BW management on shared display links (rev3) 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=ZPVTiYWRnRRyIViQ@intel.com \
--to=ville.syrjala@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox