From: "Navare, Manasi" <manasi.d.navare@intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 13/16] drm/i915: Split PPS write from DSC enable
Date: Mon, 27 Sep 2021 05:01:20 -0700 [thread overview]
Message-ID: <20210927120114.GA28419@labuser-Z97X-UD5H> (raw)
In-Reply-To: <20210913144440.23008-14-ville.syrjala@linux.intel.com>
On Mon, Sep 13, 2021 at 05:44:37PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The PPS SDP is fed into the transcoder whereas the DSC
> block is (or at least can be) per pipe. Let's split these
> into two distinct operations in an effort to untagle the
> bigjoiner mess where we have two pipes feeding a single
> transcoder.
>
Yes makes sense.
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c | 6 ++--
> drivers/gpu/drm/i915/display/intel_ddi.c | 14 +++++++--
> drivers/gpu/drm/i915/display/intel_display.c | 5 ++-
> drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
> drivers/gpu/drm/i915/display/intel_vdsc.c | 33 +++++++++-----------
> drivers/gpu/drm/i915/display/intel_vdsc.h | 10 +++---
> 6 files changed, 39 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 060bc8fb0d30..070ad144ef83 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1246,7 +1246,9 @@ static void gen11_dsi_pre_enable(struct intel_atomic_state *state,
> /* step5: program and powerup panel */
> gen11_dsi_powerup_panel(encoder);
>
> - intel_dsc_enable(encoder, pipe_config);
> + intel_dsc_dsi_pps_write(encoder, pipe_config);
> +
> + intel_dsc_enable(pipe_config);
>
> /* step6c: configure transcoder timings */
> gen11_dsi_set_transcoder_timings(encoder, pipe_config);
> @@ -1636,7 +1638,7 @@ static int gen11_dsi_dsc_compute_config(struct intel_encoder *encoder,
> /* FIXME: initialize from VBT */
> vdsc_cfg->rc_model_size = DSC_RC_MODEL_SIZE_CONST;
>
> - ret = intel_dsc_compute_params(encoder, crtc_state);
> + ret = intel_dsc_compute_params(crtc_state);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 4fbffce501dc..f51c5d732d41 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2434,7 +2434,10 @@ static void dg2_ddi_pre_enable_dp(struct intel_atomic_state *state,
>
> /* 5.k Configure and enable FEC if needed */
> intel_ddi_enable_fec(encoder, crtc_state);
> - intel_dsc_enable(encoder, crtc_state);
> +
> + intel_dsc_dp_pps_write(encoder, crtc_state);
> +
> + intel_dsc_enable(crtc_state);
> }
>
> static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
> @@ -2575,8 +2578,11 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>
> /* 7.l Configure and enable FEC if needed */
> intel_ddi_enable_fec(encoder, crtc_state);
> +
> + intel_dsc_dp_pps_write(encoder, crtc_state);
> +
> if (!crtc_state->bigjoiner)
> - intel_dsc_enable(encoder, crtc_state);
> + intel_dsc_enable(crtc_state);
> }
>
> static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
> @@ -2641,8 +2647,10 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
> if (!is_mst)
> intel_ddi_enable_pipe_clock(encoder, crtc_state);
>
> + intel_dsc_dp_pps_write(encoder, crtc_state);
> +
> if (!crtc_state->bigjoiner)
> - intel_dsc_enable(encoder, crtc_state);
> + intel_dsc_enable(crtc_state);
> }
>
> static void intel_ddi_pre_enable_dp(struct intel_atomic_state *state,
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 5716085e15f5..4e659a103984 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3007,7 +3007,7 @@ static void icl_ddi_bigjoiner_pre_enable(struct intel_atomic_state *state,
>
> if (!crtc_state->bigjoiner_slave) {
> /* need to enable VDSC, which we skipped in pre-enable */
> - intel_dsc_enable(encoder, crtc_state);
> + intel_dsc_enable(crtc_state);
> } else {
> /*
> * Enable sequence steps 1-7 on bigjoiner master
> @@ -3017,8 +3017,7 @@ static void icl_ddi_bigjoiner_pre_enable(struct intel_atomic_state *state,
> intel_enable_shared_dpll(master_crtc_state);
> intel_encoders_pre_enable(state, master_crtc);
>
> - /* and DSC on slave */
> - intel_dsc_enable(NULL, crtc_state);
> + intel_dsc_enable(crtc_state);
> }
>
> if (DISPLAY_VER(dev_priv) >= 13)
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index e898834cc5f9..68ddd9a490e2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1260,7 +1260,7 @@ static int intel_dp_dsc_compute_params(struct intel_encoder *encoder,
> else
> vdsc_cfg->slice_height = 2;
>
> - ret = intel_dsc_compute_params(encoder, crtc_state);
> + ret = intel_dsc_compute_params(crtc_state);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 2275f99ce9d7..fa84be609d5d 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -442,10 +442,10 @@ calculate_rc_params(struct rc_parameters *rc,
> }
> }
>
> -int intel_dsc_compute_params(struct intel_encoder *encoder,
> - struct intel_crtc_state *pipe_config)
> +int intel_dsc_compute_params(struct intel_crtc_state *pipe_config)
> {
> - struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> struct drm_dsc_config *vdsc_cfg = &pipe_config->dsc.config;
> u16 compressed_bpp = pipe_config->dsc.compressed_bpp;
> const struct rc_parameters *rc_params;
> @@ -1055,8 +1055,8 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> }
> }
>
> -static void intel_dsc_dsi_pps_write(struct intel_encoder *encoder,
> - const struct intel_crtc_state *crtc_state)
> +void intel_dsc_dsi_pps_write(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state)
> {
> const struct drm_dsc_config *vdsc_cfg = &crtc_state->dsc.config;
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> @@ -1064,6 +1064,9 @@ static void intel_dsc_dsi_pps_write(struct intel_encoder *encoder,
> struct drm_dsc_picture_parameter_set pps;
> enum port port;
>
> + if (!crtc_state->dsc.compression_enable)
> + return;
> +
> drm_dsc_pps_payload_pack(&pps, vdsc_cfg);
>
> for_each_dsi_port(port, intel_dsi->ports) {
> @@ -1074,14 +1077,16 @@ static void intel_dsc_dsi_pps_write(struct intel_encoder *encoder,
> }
> }
>
> -static void intel_dsc_dp_pps_write(struct intel_encoder *encoder,
> - const struct intel_crtc_state *crtc_state)
> +void intel_dsc_dp_pps_write(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state)
> {
> - struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> - struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> const struct drm_dsc_config *vdsc_cfg = &crtc_state->dsc.config;
> struct drm_dsc_pps_infoframe dp_dsc_pps_sdp;
>
> + if (!crtc_state->dsc.compression_enable)
> + return;
> +
> /* Prepare DP SDP PPS header as per DP 1.4 spec, Table 2-123 */
> drm_dsc_dp_pps_header_init(&dp_dsc_pps_sdp.pps_header);
>
> @@ -1142,8 +1147,7 @@ void intel_uncompressed_joiner_enable(const struct intel_crtc_state *crtc_state)
> }
> }
>
> -void intel_dsc_enable(struct intel_encoder *encoder,
> - const struct intel_crtc_state *crtc_state)
> +void intel_dsc_enable(const struct intel_crtc_state *crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -1155,13 +1159,6 @@ void intel_dsc_enable(struct intel_encoder *encoder,
>
> intel_dsc_pps_configure(crtc_state);
>
> - if (!crtc_state->bigjoiner_slave) {
> - if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
> - intel_dsc_dsi_pps_write(encoder, crtc_state);
> - else
> - intel_dsc_dp_pps_write(encoder, crtc_state);
> - }
> -
> dss_ctl2_val |= LEFT_BRANCH_VDSC_ENABLE;
> if (crtc_state->dsc.dsc_split) {
> dss_ctl2_val |= RIGHT_BRANCH_VDSC_ENABLE;
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.h b/drivers/gpu/drm/i915/display/intel_vdsc.h
> index 0c5d80a572da..4ec75f715986 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.h
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.h
> @@ -15,15 +15,17 @@ struct intel_encoder;
>
> bool intel_dsc_source_support(const struct intel_crtc_state *crtc_state);
> void intel_uncompressed_joiner_enable(const struct intel_crtc_state *crtc_state);
> -void intel_dsc_enable(struct intel_encoder *encoder,
> - const struct intel_crtc_state *crtc_state);
> +void intel_dsc_enable(const struct intel_crtc_state *crtc_state);
> void intel_dsc_disable(const struct intel_crtc_state *crtc_state);
> -int intel_dsc_compute_params(struct intel_encoder *encoder,
> - struct intel_crtc_state *pipe_config);
> +int intel_dsc_compute_params(struct intel_crtc_state *pipe_config);
> void intel_uncompressed_joiner_get_config(struct intel_crtc_state *crtc_state);
> void intel_dsc_get_config(struct intel_crtc_state *crtc_state);
> enum intel_display_power_domain
> intel_dsc_power_domain(struct intel_crtc *crtc, enum transcoder cpu_transcoder);
> struct intel_crtc *intel_dsc_get_bigjoiner_secondary(const struct intel_crtc *primary_crtc);
> +void intel_dsc_dsi_pps_write(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state);
> +void intel_dsc_dp_pps_write(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state);
>
> #endif /* __INTEL_VDSC_H__ */
> --
> 2.32.0
>
next prev parent reply other threads:[~2021-09-27 11:49 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-13 14:44 [Intel-gfx] [PATCH 00/16] drm/i915: Fix bigjoiner state readout Ville Syrjala
2021-09-13 14:44 ` [Intel-gfx] [PATCH 01/16] Revert "drm/i915/display: Disable audio, DRRS and PSR before planes" Ville Syrjala
2021-09-13 16:28 ` Souza, Jose
2021-09-14 8:20 ` Ville Syrjälä
2021-09-14 23:24 ` Souza, Jose
2021-09-15 0:00 ` Souza, Jose
2021-09-15 12:30 ` Ville Syrjälä
2021-09-15 20:19 ` Souza, Jose
2021-09-16 13:21 ` Ville Syrjälä
2021-09-17 0:14 ` Souza, Jose
2021-09-13 14:44 ` [Intel-gfx] [PATCH 02/16] drm/i915: Disable all planes before modesetting any pipes Ville Syrjala
2021-09-20 7:52 ` Navare, Manasi
2021-09-21 12:49 ` Ville Syrjälä
2021-09-27 11:20 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 03/16] drm/i915: Extract intel_dp_use_bigjoiner() Ville Syrjala
2021-09-15 10:12 ` Jani Nikula
2021-09-15 15:39 ` Ville Syrjälä
2021-09-21 11:10 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 04/16] drm/i915: Flatten hsw_crtc_compute_clock() Ville Syrjala
2021-09-15 10:13 ` Jani Nikula
2021-09-13 14:44 ` [Intel-gfx] [PATCH 05/16] drm/i915: s/pipe/transcoder/ when dealing with PIPECONF/TRANSCONF Ville Syrjala
2021-09-15 10:16 ` Jani Nikula
2021-09-15 13:00 ` Ville Syrjälä
2021-09-15 13:17 ` Jani Nikula
2021-09-13 14:44 ` [Intel-gfx] [PATCH 06/16] drm/i915: Introduce with_intel_display_power_if_enabled() Ville Syrjala
2021-09-15 13:22 ` Jani Nikula
2021-09-13 14:44 ` [Intel-gfx] [PATCH 07/16] drm/i915: Adjust intel_dsc_power_domain() calling convention Ville Syrjala
2021-09-15 10:19 ` Jani Nikula
2021-09-13 14:44 ` [Intel-gfx] [PATCH 08/16] drm/i915: Extract hsw_panel_transcoders() Ville Syrjala
2021-09-15 10:20 ` Jani Nikula
2021-09-13 14:44 ` [Intel-gfx] [PATCH 09/16] drm/i915: Pimp HSW+ transcoder state readout Ville Syrjala
2021-09-21 11:46 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 10/16] drm/i915: Configure TRANSCONF just the once with bigjoiner Ville Syrjala
2021-09-21 11:51 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 11/16] drm/i915: Introduce intel_master_crtc() Ville Syrjala
2021-09-15 10:24 ` Jani Nikula
2021-09-15 12:21 ` Ville Syrjälä
2021-10-21 23:27 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 12/16] drm/i915: Simplify intel_crtc_copy_uapi_to_hw_state_nomodeset() Ville Syrjala
2021-09-27 11:27 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 13/16] drm/i915: Split PPS write from DSC enable Ville Syrjala
2021-09-27 12:01 ` Navare, Manasi [this message]
2021-09-13 14:44 ` [Intel-gfx] [PATCH 14/16] drm/i915: Perform correct cpu_transcoder readout for bigjoiner Ville Syrjala
2021-10-20 20:35 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 15/16] drm/i915: Reduce bigjoiner special casing Ville Syrjala
2021-10-20 23:52 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 16/16] drm/i915: Nuke PIPE_CONFIG_QUIRK_BIGJOINER_SLAVE Ville Syrjala
2021-10-20 23:53 ` Navare, Manasi
2021-09-13 16:05 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix bigjoiner state readout Patchwork
2021-09-13 16:34 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-09-13 18:39 ` [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=20210927120114.GA28419@labuser-Z97X-UD5H \
--to=manasi.d.navare@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox