From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Manasi Navare <manasi.d.navare@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
Anusha Srivatsa <anusha.srivatsa@intel.com>,
dri-devel@lists.freedesktop.org,
Gaurav K Singh <gaurav.k.singh@intel.com>
Subject: Re: [PATCH v10 08/23] drm/i915/dp: Compute DSC pipe config in atomic check
Date: Tue, 27 Nov 2018 15:57:23 +0200 [thread overview]
Message-ID: <20181127135723.GW9144@intel.com> (raw)
In-Reply-To: <20181120183736.28054-9-manasi.d.navare@intel.com>
On Tue, Nov 20, 2018 at 10:37:21AM -0800, Manasi Navare wrote:
> DSC params like the enable, compressed bpp, slice count and
> dsc_split are added to the intel_crtc_state. These parameters
> are set based on the requested mode and available link parameters
> during the pipe configuration in atomic check phase.
> These values are then later used to populate the remaining DSC
> and RC parameters before enbaling DSC in atomic commit.
>
> v14:
> Remove leftovers, use dsc_bpc, refine dsc_compute_config (Ville)
> v13:
> * Compute DSC bpc only when DSC is req to be enabled (Ville)
> v12:
> * Override bpp with dsc dpcd color depth (Manasi)
> v11:
> * Const crtc_state, reject DSC on DP without FEC (Ville)
> * Dont set dsc_split to false (Ville)
> v10:
> * Add a helper for dp_dsc support (Ville)
> * Set pipe_config to max bpp, link params for DSC for now (Ville)
> * Compute bpp - use dp dsc support helper (Ville)
> v9:
> * Rebase on top of drm-tip that now uses fast_narrow config
> for edp (Manasi)
> v8:
> * Check for DSC bpc not 0 (manasi)
>
> v7:
> * Fix indentation in compute_m_n (Manasi)
>
> v6 (From Gaurav):
> * Remove function call of intel_dp_compute_dsc_params() and
> invoke intel_dp_compute_dsc_params() in the patch where
> it is defined to fix compilation warning (Gaurav)
>
> v5:
> Add drm_dsc_cfg in intel_crtc_state (Manasi)
>
> v4:
> * Rebase on refactoring of intel_dp_compute_config on tip (Manasi)
> * Add a comment why we need to check PSR while enabling DSC (Gaurav)
>
> v3:
> * Check PPR > max_cdclock to use 2 VDSC instances (Ville)
>
> v2:
> * Add if-else for eDP/DP (Gaurav)
>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Acked-by: Jani Nikula <jani.nikula@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> drivers/gpu/drm/i915/intel_display.h | 2 +-
> drivers/gpu/drm/i915/intel_dp.c | 191 ++++++++++++++++++++++++---
> 3 files changed, 171 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 132e978227fb..f2e6425d09ef 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6723,7 +6723,7 @@ static void compute_m_n(unsigned int m, unsigned int n,
> }
>
> void
> -intel_link_compute_m_n(int bits_per_pixel, int nlanes,
> +intel_link_compute_m_n(u16 bits_per_pixel, int nlanes,
> int pixel_clock, int link_clock,
> struct intel_link_m_n *m_n,
> bool constant_n)
> diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> index 5d50decbcbb5..afb6435887df 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -407,7 +407,7 @@ struct intel_link_m_n {
> (__i)++) \
> for_each_if(plane)
>
> -void intel_link_compute_m_n(int bpp, int nlanes,
> +void intel_link_compute_m_n(u16 bpp, int nlanes,
> int pixel_clock, int link_clock,
> struct intel_link_m_n *m_n,
> bool constant_n);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 2b090609bee2..78ec775aa90a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -47,6 +47,8 @@
>
> /* DP DSC small joiner has 2 FIFOs each of 640 x 6 bytes */
> #define DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER 61440
> +#define DP_DSC_MIN_SUPPORTED_BPC 8
> +#define DP_DSC_MAX_SUPPORTED_BPC 10
>
> /* DP DSC throughput values used for slice count calculations KPixels/s */
> #define DP_DSC_PEAK_PIXEL_RATE 2720000
> @@ -1708,6 +1710,26 @@ struct link_config_limits {
> int min_bpp, max_bpp;
> };
>
> +static bool intel_dp_source_supports_dsc(struct intel_dp *intel_dp,
> + const struct intel_crtc_state *pipe_config)
> +{
> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +
> + /* FIXME: FEC needed for external DP until then reject DSC on DP */
> + if (!intel_dp_is_edp(intel_dp))
> + return false;
> +
> + return INTEL_GEN(dev_priv) >= 10 &&
> + pipe_config->cpu_transcoder != TRANSCODER_A;
> +}
> +
> +static bool intel_dp_supports_dsc(struct intel_dp *intel_dp,
> + const struct intel_crtc_state *pipe_config)
> +{
> + return intel_dp_source_supports_dsc(intel_dp, pipe_config) &&
> + drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd);
> +}
> +
> static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
> struct intel_crtc_state *pipe_config)
> {
> @@ -1842,14 +1864,114 @@ intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
> return false;
> }
>
> +static int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 dsc_max_bpc)
> +{
> + int i, num_bpc;
> + u8 dsc_bpc[3] = {0};
> +
> + num_bpc = drm_dp_dsc_sink_supported_input_bpcs(intel_dp->dsc_dpcd,
> + dsc_bpc);
> + for (i = 0; i < num_bpc; i++) {
> + if (dsc_max_bpc >= dsc_bpc[i])
> + return dsc_bpc[i] * 3;
> + }
> +
> + return 0;
> +}
> +
> +static bool intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> + struct intel_crtc_state *pipe_config,
> + struct drm_connector_state *conn_state,
> + struct link_config_limits *limits)
> +{
> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> + struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> + struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> + u8 dsc_max_bpc;
> + u16 dsc_max_output_bpp = 0;
> + u8 dsc_dp_slice_count = 0;
Pointless initialization, and these can be moved to narrower scope.
> + int pipe_bpp;
> +
> + if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> + return false;
> +
> + dsc_max_bpc = min_t(u8, DP_DSC_MAX_SUPPORTED_BPC,
> + conn_state->max_requested_bpc);
> +
> + pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, dsc_max_bpc);
> + if (pipe_bpp < DP_DSC_MIN_SUPPORTED_BPC * 3) {
> + DRM_DEBUG_KMS("No DSC support for less than 8bpc\n");
> + return false;
> + }
> +
> + /*
> + * For now enable DSC for max bpp, max link rate, max lane count.
> + * Optimize this later for the minimum possible link rate/lane count
> + * with DSC enabled for the requested mode.
> + */
> + pipe_config->pipe_bpp = pipe_bpp;
> + pipe_config->port_clock = intel_dp->common_rates[limits->max_clock];
> + pipe_config->lane_count = limits->max_lane_count;
> +
> + if (intel_dp_is_edp(intel_dp)) {
> + pipe_config->dsc_params.compressed_bpp =
> + min_t(u16, drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4,
> + pipe_config->pipe_bpp);
> + pipe_config->dsc_params.slice_count =
> + drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> + true);
> + } else {
> + dsc_max_output_bpp =
> + intel_dp_dsc_get_output_bpp(pipe_config->port_clock,
> + pipe_config->lane_count,
> + adjusted_mode->crtc_clock,
> + adjusted_mode->crtc_hdisplay);
> + dsc_dp_slice_count =
> + intel_dp_dsc_get_slice_count(intel_dp,
> + adjusted_mode->crtc_clock,
> + adjusted_mode->crtc_hdisplay);
There's some kind of indent fail here.
> + if (!(dsc_max_output_bpp && dsc_dp_slice_count)) {
'!a || !b' might be a bit more legible.
The logic seems good now so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> + DRM_DEBUG_KMS("Compressed BPP/Slice Count not supported\n");
> + return false;
> + }
> + pipe_config->dsc_params.compressed_bpp = min_t(u16,
> + dsc_max_output_bpp >> 4,
> + pipe_config->pipe_bpp);
> + pipe_config->dsc_params.slice_count = dsc_dp_slice_count;
> + }
> + /*
> + * VDSC engine operates at 1 Pixel per clock, so if peak pixel rate
> + * is greater than the maximum Cdclock and if slice count is even
> + * then we need to use 2 VDSC instances.
> + */
> + if (adjusted_mode->crtc_clock > dev_priv->max_cdclk_freq) {
> + if (pipe_config->dsc_params.slice_count > 1) {
> + pipe_config->dsc_params.dsc_split = true;
> + } else {
> + DRM_DEBUG_KMS("Cannot split stream to use 2 VDSC instances\n");
> + return false;
> + }
> + }
> + pipe_config->dsc_params.compression_enable = true;
> + DRM_DEBUG_KMS("DP DSC computed with Input Bpp = %d "
> + "Compressed Bpp = %d Slice Count = %d\n",
> + pipe_config->pipe_bpp,
> + pipe_config->dsc_params.compressed_bpp,
> + pipe_config->dsc_params.slice_count);
> +
> + return true;
> +}
> +
> static bool
> intel_dp_compute_link_config(struct intel_encoder *encoder,
> - struct intel_crtc_state *pipe_config)
> + struct intel_crtc_state *pipe_config,
> + struct drm_connector_state *conn_state)
> {
> struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> struct link_config_limits limits;
> int common_len;
> + bool ret;
>
> common_len = intel_dp_common_len_rate_limit(intel_dp,
> intel_dp->max_link_rate);
> @@ -1888,7 +2010,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
> intel_dp->common_rates[limits.max_clock],
> limits.max_bpp, adjusted_mode->crtc_clock);
>
> - if (intel_dp_is_edp(intel_dp)) {
> + if (intel_dp_is_edp(intel_dp))
> /*
> * Optimize for fast and narrow. eDP 1.3 section 3.3 and eDP 1.4
> * section A.1: "It is recommended that the minimum number of
> @@ -1898,26 +2020,42 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
> * Note that we use the max clock and lane count for eDP 1.3 and
> * earlier, and fast vs. wide is irrelevant.
> */
> - if (!intel_dp_compute_link_config_fast(intel_dp, pipe_config,
> - &limits))
> - return false;
> - } else {
> + ret = intel_dp_compute_link_config_fast(intel_dp, pipe_config,
> + &limits);
> + else
> /* Optimize for slow and wide. */
> - if (!intel_dp_compute_link_config_wide(intel_dp, pipe_config,
> - &limits))
> + ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config,
> + &limits);
> +
> + /* enable compression if the mode doesn't fit available BW */
> + if (!ret) {
> + if (!intel_dp_dsc_compute_config(intel_dp, pipe_config,
> + conn_state, &limits))
> return false;
> }
>
> - DRM_DEBUG_KMS("DP lane count %d clock %d bpp %d\n",
> - pipe_config->lane_count, pipe_config->port_clock,
> - pipe_config->pipe_bpp);
> + if (pipe_config->dsc_params.compression_enable) {
> + DRM_DEBUG_KMS("DP lane count %d clock %d Input bpp %d Compressed bpp %d\n",
> + pipe_config->lane_count, pipe_config->port_clock,
> + pipe_config->pipe_bpp,
> + pipe_config->dsc_params.compressed_bpp);
>
> - DRM_DEBUG_KMS("DP link rate required %i available %i\n",
> - intel_dp_link_required(adjusted_mode->crtc_clock,
> - pipe_config->pipe_bpp),
> - intel_dp_max_data_rate(pipe_config->port_clock,
> - pipe_config->lane_count));
> + DRM_DEBUG_KMS("DP link rate required %i available %i\n",
> + intel_dp_link_required(adjusted_mode->crtc_clock,
> + pipe_config->dsc_params.compressed_bpp),
> + intel_dp_max_data_rate(pipe_config->port_clock,
> + pipe_config->lane_count));
> + } else {
> + DRM_DEBUG_KMS("DP lane count %d clock %d bpp %d\n",
> + pipe_config->lane_count, pipe_config->port_clock,
> + pipe_config->pipe_bpp);
>
> + DRM_DEBUG_KMS("DP link rate required %i available %i\n",
> + intel_dp_link_required(adjusted_mode->crtc_clock,
> + pipe_config->pipe_bpp),
> + intel_dp_max_data_rate(pipe_config->port_clock,
> + pipe_config->lane_count));
> + }
> return true;
> }
>
> @@ -1983,7 +2121,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> return false;
>
> - if (!intel_dp_compute_link_config(encoder, pipe_config))
> + if (!intel_dp_compute_link_config(encoder, pipe_config, conn_state))
> return false;
>
> if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) {
> @@ -2001,11 +2139,20 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
> }
>
> - intel_link_compute_m_n(pipe_config->pipe_bpp, pipe_config->lane_count,
> - adjusted_mode->crtc_clock,
> - pipe_config->port_clock,
> - &pipe_config->dp_m_n,
> - constant_n);
> + if (!pipe_config->dsc_params.compression_enable)
> + intel_link_compute_m_n(pipe_config->pipe_bpp,
> + pipe_config->lane_count,
> + adjusted_mode->crtc_clock,
> + pipe_config->port_clock,
> + &pipe_config->dp_m_n,
> + constant_n);
> + else
> + intel_link_compute_m_n(pipe_config->dsc_params.compression_enable,
> + pipe_config->lane_count,
> + adjusted_mode->crtc_clock,
> + pipe_config->port_clock,
> + &pipe_config->dp_m_n,
> + constant_n);
>
> if (intel_connector->panel.downclock_mode != NULL &&
> dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> --
> 2.19.1
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2018-11-27 13:57 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-20 18:37 [PATCH v10 00/23] Respin of remaining DSC + FEC patches Manasi Navare
2018-11-20 18:37 ` [PATCH v10 01/23] drm/dsc: Modify DRM helper to return complete DSC color depth capabilities Manasi Navare
2018-11-27 13:47 ` Ville Syrjälä
2018-11-27 17:44 ` Srivatsa, Anusha
2018-11-20 18:37 ` [PATCH v10 02/23] drm/dsc: Define Display Stream Compression PPS infoframe Manasi Navare
2018-11-20 18:37 ` [PATCH v10 03/23] drm/dsc: Define VESA Display Stream Compression Capabilities Manasi Navare
2018-11-20 18:37 ` [PATCH v10 04/23] drm/dsc: Define Rate Control values that do not change over configurations Manasi Navare
2018-11-20 18:37 ` [PATCH v10 05/23] drm/dsc: Add helpers for DSC picture parameter set infoframes Manasi Navare
2018-11-20 18:37 ` [PATCH v10 06/23] drm/dsc: Define the DSC 1.1 and 1.2 Line Buffer depth constants Manasi Navare
2018-11-20 18:37 ` [PATCH v10 07/23] drm/i915/dp: Add DSC params and DSC config to intel_crtc_state Manasi Navare
2018-11-20 18:37 ` [PATCH v10 08/23] drm/i915/dp: Compute DSC pipe config in atomic check Manasi Navare
2018-11-27 13:57 ` Ville Syrjälä [this message]
2018-11-27 17:29 ` Manasi Navare
2018-11-20 18:37 ` [PATCH v10 09/23] drm/i915/dp: Do not enable PSR2 if DSC is enabled Manasi Navare
2018-11-20 18:37 ` [PATCH v10 10/23] drm/i915/dsc: Define & Compute VESA DSC params Manasi Navare
2018-11-20 18:37 ` [PATCH v10 11/23] drm/i915/dsc: Compute Rate Control parameters for DSC Manasi Navare
2018-11-20 18:37 ` [PATCH v10 12/23] drm/i915/dp: Enable/Disable DSC in DP Sink Manasi Navare
2018-11-20 18:37 ` [PATCH v10 13/23] drm/i915/dsc: Add a power domain for VDSC on eDP/MIPI DSI Manasi Navare
2018-11-20 18:37 ` [PATCH v10 14/23] drm/i915/dp: Configure i915 Picture parameter Set registers during DSC enabling Manasi Navare
2018-11-20 18:37 ` [PATCH v10 15/23] drm/i915/dp: Use the existing write_infoframe() for DSC PPS SDPs Manasi Navare
2018-11-20 18:37 ` [PATCH v10 16/23] drm/i915/dp: Populate DSC PPS SDP and send PPS infoframes Manasi Navare
2018-11-20 18:37 ` [PATCH v10 17/23] drm/i915/dp: Configure Display stream splitter registers during DSC enable Manasi Navare
2018-11-20 18:37 ` [PATCH v10 18/23] drm/i915/dp: Disable DSC in source by disabling DSS CTL bits Manasi Navare
2018-11-20 18:37 ` [PATCH v10 19/23] drm/i915/dsc: Enable and disable appropriate power wells for VDSC Manasi Navare
2018-11-20 18:37 ` [PATCH v10 20/23] i915/dp/fec: Add fec_enable to the crtc state Manasi Navare
2018-11-27 17:46 ` Manasi Navare
2018-11-20 18:37 ` [PATCH v10 21/23] drm/i915/fec: Set FEC_READY in FEC_CONFIGURATION Manasi Navare
2018-11-20 18:37 ` [PATCH v10 22/23] i915/dp/fec: Configure the Forward Error Correction bits Manasi Navare
2018-11-27 17:47 ` Manasi Navare
2018-11-20 18:37 ` [PATCH v10 23/23] drm/i915/fec: Disable FEC state Manasi Navare
2018-11-20 18:58 ` ✗ Fi.CI.CHECKPATCH: warning for Respin of remaining DSC + FEC patches Patchwork
2018-11-20 19:06 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-11-20 19:29 ` ✓ Fi.CI.BAT: success " Patchwork
2018-11-21 6:54 ` ✓ Fi.CI.IGT: " Patchwork
2018-11-29 20:57 ` [PATCH v10 00/23] " Manasi Navare
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=20181127135723.GW9144@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=anusha.srivatsa@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=gaurav.k.singh@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=manasi.d.navare@intel.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 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.