From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 13/13] drm/i915/dp: Use consistent name for link bpp and compressed bpp
Date: Mon, 24 Apr 2023 16:04:39 +0300 [thread overview]
Message-ID: <ZEZ+Z5AaeX8XtpJa@intel.com> (raw)
In-Reply-To: <20230331101613.936776-14-ankit.k.nautiyal@intel.com>
On Fri, Mar 31, 2023 at 03:46:13PM +0530, Ankit Nautiyal wrote:
> Currently there are many places where we use output_bpp for link bpp and
> compressed bpp.
> Lets use consistent naming:
> output_bpp : The intermediate value taking into account the
> output_format chroma subsampling.
> compressed_bpp : target bpp for the DSC encoder.
> link_bpp : final bpp used in the link.
>
> For 444 sampling without DSC:
> link_bpp = output_bpp = pipe_bpp
>
> For 420 sampling without DSC:
> output_bpp = pipe_bpp / 2
> link_bpp = output_bpp
>
> For 444 sampling with DSC:
> output_bpp = pipe_bpp
> link_bpp = compressed_bpp, computed with output_bpp (i.e. pipe_bpp in
> this case)
>
> For 420 sampling with DSC:
> output_bpp = pipe_bpp/2
> link_bpp = compressed_bpp, computed with output_bpp
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 84 ++++++++++-----------
> drivers/gpu/drm/i915/display/intel_dp.h | 14 ++--
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 22 +++---
> 3 files changed, 60 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 63f3f6ff2cad..22b98653d539 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -710,13 +710,13 @@ u32 intel_dp_dsc_nearest_valid_bpp(struct drm_i915_private *i915, u32 bpp, u32 p
> return bits_per_pixel;
> }
>
> -u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
> - u32 link_clock, u32 lane_count,
> - u32 mode_clock, u32 mode_hdisplay,
> - bool bigjoiner,
> - enum intel_output_format output_format,
> - u32 pipe_bpp,
> - u32 timeslots)
> +u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
> + u32 link_clock, u32 lane_count,
> + u32 mode_clock, u32 mode_hdisplay,
> + bool bigjoiner,
> + enum intel_output_format output_format,
> + u32 pipe_bpp,
> + u32 timeslots)
> {
> u32 bits_per_pixel, max_bpp_small_joiner_ram;
>
> @@ -1118,7 +1118,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
> int target_clock = mode->clock;
> int max_rate, mode_rate, max_lanes, max_link_clock;
> int max_dotclk = dev_priv->max_dotclk_freq;
> - u16 dsc_max_output_bpp = 0;
> + u16 dsc_max_compressed_bpp = 0;
> u8 dsc_slice_count = 0;
> enum drm_mode_status status;
> bool dsc = false, bigjoiner = false;
> @@ -1173,21 +1173,21 @@ intel_dp_mode_valid(struct drm_connector *_connector,
> * integer value since we support only integer values of bpp.
> */
> if (intel_dp_is_edp(intel_dp)) {
> - dsc_max_output_bpp =
> + dsc_max_compressed_bpp =
> drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4;
Renaming that as well would be nice, as a separate patch.
> dsc_slice_count =
> drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> true);
> } else if (drm_dp_sink_supports_fec(intel_dp->fec_capable)) {
> - dsc_max_output_bpp =
> - intel_dp_dsc_get_output_bpp(dev_priv,
> - max_link_clock,
> - max_lanes,
> - target_clock,
> - mode->hdisplay,
> - bigjoiner,
> - output_format,
> - pipe_bpp, 64) >> 4;
> + dsc_max_compressed_bpp =
> + intel_dp_dsc_get_max_compressed_bpp(dev_priv,
> + max_link_clock,
> + max_lanes,
> + target_clock,
> + mode->hdisplay,
> + bigjoiner,
> + output_format,
> + pipe_bpp, 64) >> 4;
> dsc_slice_count =
> intel_dp_dsc_get_slice_count(intel_dp,
> target_clock,
> @@ -1195,7 +1195,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
> bigjoiner);
> }
>
> - dsc = dsc_max_output_bpp && dsc_slice_count;
> + dsc = dsc_max_compressed_bpp && dsc_slice_count;
> }
>
> /*
> @@ -1477,9 +1477,9 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
> int mode_rate, link_rate, link_avail;
>
> for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
> - int output_bpp = intel_dp_output_bpp(pipe_config->output_format, bpp);
> + int link_bpp = intel_dp_output_bpp(pipe_config->output_format, bpp);
>
> - mode_rate = intel_dp_link_required(clock, output_bpp);
> + mode_rate = intel_dp_link_required(clock, link_bpp);
>
> for (i = 0; i < intel_dp->num_common_rates; i++) {
> link_rate = intel_dp_common_rate(intel_dp, i);
> @@ -1728,21 +1728,21 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> true);
> } else {
> - u16 dsc_max_output_bpp = 0;
> + u16 dsc_max_compressed_bpp = 0;
> u8 dsc_dp_slice_count;
>
> if (compute_pipe_bpp) {
> - dsc_max_output_bpp =
> - intel_dp_dsc_get_output_bpp(dev_priv,
> - pipe_config->port_clock,
> - pipe_config->lane_count,
> - adjusted_mode->crtc_clock,
> - adjusted_mode->crtc_hdisplay,
> - pipe_config->bigjoiner_pipes,
> - pipe_config->output_format,
> - pipe_bpp,
> - timeslots);
> - if (!dsc_max_output_bpp) {
> + dsc_max_compressed_bpp =
> + intel_dp_dsc_get_max_compressed_bpp(dev_priv,
> + pipe_config->port_clock,
> + pipe_config->lane_count,
> + adjusted_mode->crtc_clock,
> + adjusted_mode->crtc_hdisplay,
> + pipe_config->bigjoiner_pipes,
> + pipe_config->output_format,
> + pipe_bpp,
> + timeslots);
> + if (!dsc_max_compressed_bpp) {
> drm_dbg_kms(&dev_priv->drm,
> "Compressed BPP not supported\n");
> return -EINVAL;
> @@ -1767,7 +1767,7 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> */
> if (compute_pipe_bpp) {
> pipe_config->dsc.compressed_bpp = min_t(u16,
> - dsc_max_output_bpp >> 4,
> + dsc_max_compressed_bpp >> 4,
> pipe_config->pipe_bpp);
Should that be min(max_compressed_bpp>>4, output_bpp) ?
The rest makes sense
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> }
> pipe_config->dsc.slice_count = dsc_dp_slice_count;
> @@ -2143,7 +2143,7 @@ static bool can_enable_drrs(struct intel_connector *connector,
> static void
> intel_dp_drrs_compute_config(struct intel_connector *connector,
> struct intel_crtc_state *pipe_config,
> - int output_bpp)
> + int link_bpp)
> {
> struct drm_i915_private *i915 = to_i915(connector->base.dev);
> const struct drm_display_mode *downclock_mode =
> @@ -2168,7 +2168,7 @@ intel_dp_drrs_compute_config(struct intel_connector *connector,
> if (pipe_config->splitter.enable)
> pixel_clock /= pipe_config->splitter.link_count;
>
> - intel_link_compute_m_n(output_bpp, pipe_config->lane_count, pixel_clock,
> + intel_link_compute_m_n(link_bpp, pipe_config->lane_count, pixel_clock,
> pipe_config->port_clock, &pipe_config->dp_m2_n2,
> pipe_config->fec_enable);
>
> @@ -2265,7 +2265,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> const struct drm_display_mode *fixed_mode;
> struct intel_connector *connector = intel_dp->attached_connector;
> - int ret = 0, output_bpp;
> + int ret = 0, link_bpp;
>
> if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && encoder->port != PORT_A)
> pipe_config->has_pch_encoder = true;
> @@ -2315,10 +2315,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> intel_dp_limited_color_range(pipe_config, conn_state);
>
> if (pipe_config->dsc.compression_enable)
> - output_bpp = pipe_config->dsc.compressed_bpp;
> + link_bpp = pipe_config->dsc.compressed_bpp;
> else
> - output_bpp = intel_dp_output_bpp(pipe_config->output_format,
> - pipe_config->pipe_bpp);
> + link_bpp = intel_dp_output_bpp(pipe_config->output_format,
> + pipe_config->pipe_bpp);
>
> if (intel_dp->mso_link_count) {
> int n = intel_dp->mso_link_count;
> @@ -2342,7 +2342,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>
> intel_dp_audio_compute_config(encoder, pipe_config, conn_state);
>
> - intel_link_compute_m_n(output_bpp,
> + intel_link_compute_m_n(link_bpp,
> pipe_config->lane_count,
> adjusted_mode->crtc_clock,
> pipe_config->port_clock,
> @@ -2358,7 +2358,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>
> intel_vrr_compute_config(pipe_config, conn_state);
> intel_psr_compute_config(intel_dp, pipe_config, conn_state);
> - intel_dp_drrs_compute_config(connector, pipe_config, output_bpp);
> + intel_dp_drrs_compute_config(connector, pipe_config, link_bpp);
> intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
> intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 856172bfa13e..42b98546c140 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -103,13 +103,13 @@ void intel_read_dp_sdp(struct intel_encoder *encoder,
> unsigned int type);
> bool intel_digital_port_connected(struct intel_encoder *encoder);
> int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 dsc_max_bpc);
> -u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
> - u32 link_clock, u32 lane_count,
> - u32 mode_clock, u32 mode_hdisplay,
> - bool bigjoiner,
> - enum intel_output_format output_format,
> - u32 pipe_bpp,
> - u32 timeslots);
> +u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
> + u32 link_clock, u32 lane_count,
> + u32 mode_clock, u32 mode_hdisplay,
> + bool bigjoiner,
> + enum intel_output_format output_format,
> + u32 pipe_bpp,
> + u32 timeslots);
> u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> int mode_clock, int mode_hdisplay,
> bool bigjoiner);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index fec3f310fc9b..7b91c6c9c286 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -869,7 +869,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
> int max_rate, mode_rate, max_lanes, max_link_clock;
> int ret;
> bool dsc = false, bigjoiner = false;
> - u16 dsc_max_output_bpp = 0;
> + u16 dsc_max_compressed_bpp = 0;
> u8 dsc_slice_count = 0;
> int target_clock = mode->clock;
>
> @@ -923,15 +923,15 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
> int pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, U8_MAX);
>
> if (drm_dp_sink_supports_fec(intel_dp->fec_capable)) {
> - dsc_max_output_bpp =
> - intel_dp_dsc_get_output_bpp(dev_priv,
> - max_link_clock,
> - max_lanes,
> - target_clock,
> - mode->hdisplay,
> - bigjoiner,
> - INTEL_OUTPUT_FORMAT_RGB,
> - pipe_bpp, 64) >> 4;
> + dsc_max_compressed_bpp =
> + intel_dp_dsc_get_max_compressed_bpp(dev_priv,
> + max_link_clock,
> + max_lanes,
> + target_clock,
> + mode->hdisplay,
> + bigjoiner,
> + INTEL_OUTPUT_FORMAT_RGB,
> + pipe_bpp, 64) >> 4;
> dsc_slice_count =
> intel_dp_dsc_get_slice_count(intel_dp,
> target_clock,
> @@ -939,7 +939,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
> bigjoiner);
> }
>
> - dsc = dsc_max_output_bpp && dsc_slice_count;
> + dsc = dsc_max_compressed_bpp && dsc_slice_count;
> }
>
> /*
> --
> 2.25.1
--
Ville Syrjälä
Intel
next prev parent reply other threads:[~2023-04-24 13:04 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-31 10:16 [Intel-gfx] [PATCH 00/13] Handle BPC for HDMI2.1 PCON without DSC1.2 sink and other fixes Ankit Nautiyal
2023-03-31 10:16 ` [Intel-gfx] [PATCH 01/13] drm/i915/display: Add new member to configure PCON color conversion Ankit Nautiyal
2023-03-31 10:16 ` [Intel-gfx] [PATCH 02/13] drm/i915/display: Add new member in intel_dp to store ycbcr420 passthrough cap Ankit Nautiyal
2023-04-24 12:46 ` Ville Syrjälä
2023-04-26 4:53 ` Nautiyal, Ankit K
2023-03-31 10:16 ` [Intel-gfx] [PATCH 03/13] drm/i915/dp: Replace intel_dp.dfp members with the new crtc_state sink_format Ankit Nautiyal
2023-04-24 12:31 ` Ville Syrjälä
2023-04-26 4:58 ` Nautiyal, Ankit K
2023-03-31 10:16 ` [Intel-gfx] [PATCH 04/13] drm/i915/dp: Configure PCON for conversion of output_format to YCbCr444 Ankit Nautiyal
2023-04-24 12:32 ` Ville Syrjälä
2023-03-31 10:16 ` [Intel-gfx] [PATCH 05/13] drm/i915/display: Use sink_format instead of ycbcr420_output flag Ankit Nautiyal
2023-04-24 12:37 ` Ville Syrjälä
2023-04-26 5:09 ` Nautiyal, Ankit K
2023-03-31 10:16 ` [Intel-gfx] [PATCH 06/13] drm/i915/dp: Add helper to get sink_format Ankit Nautiyal
2023-04-24 12:38 ` Ville Syrjälä
2023-03-31 10:16 ` [Intel-gfx] [PATCH 07/13] drm/i915/dp: Rearrange check for illegal mode and comments in mode_valid Ankit Nautiyal
2023-03-31 10:16 ` [Intel-gfx] [PATCH 08/13] drm/i915/dp: Consider output_format while computing dsc bpp Ankit Nautiyal
2023-04-24 12:51 ` Ville Syrjälä
2023-04-26 5:31 ` Nautiyal, Ankit K
2023-03-31 10:16 ` [Intel-gfx] [PATCH 09/13] drm/i915/dp_mst: Use output_format to get the final link bpp Ankit Nautiyal
2023-04-24 12:58 ` Ville Syrjälä
2023-03-31 10:16 ` [Intel-gfx] [PATCH 10/13] drm/i915/dp: Handle BPP where HDMI2.1 DFP doesn't support DSC Ankit Nautiyal
2023-03-31 10:16 ` [Intel-gfx] [PATCH 11/13] drm/i915/dp: Fix FRL BW check for HDMI2.1 DFP Ankit Nautiyal
2023-03-31 10:16 ` [Intel-gfx] [PATCH 12/13] drm/i915/dp: Add a wrapper to check frl/tmds downstream constraints Ankit Nautiyal
2023-03-31 10:16 ` [Intel-gfx] [PATCH 13/13] drm/i915/dp: Use consistent name for link bpp and compressed bpp Ankit Nautiyal
2023-04-24 13:04 ` Ville Syrjälä [this message]
2023-04-26 6:10 ` Nautiyal, Ankit K
2023-03-31 16:20 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Handle BPC for HDMI2.1 PCON without DSC1.2 sink and other fixes (rev13) Patchwork
2023-03-31 16:20 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-03-31 16:32 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-04-01 16:09 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2023-04-24 13:09 ` [Intel-gfx] [PATCH 00/13] Handle BPC for HDMI2.1 PCON without DSC1.2 sink and other fixes Ville Syrjälä
2023-04-25 7:30 ` Nautiyal, Ankit K
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=ZEZ+Z5AaeX8XtpJa@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=ankit.k.nautiyal@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.