From: "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>
To: Suraj Kandpal <suraj.kandpal@intel.com>,
<intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH v6 3/8] drm/i915/vdsc: Add func to get no. of vdsc instances per pipe
Date: Tue, 8 Aug 2023 13:39:13 +0530 [thread overview]
Message-ID: <cdbb8362-cc65-bed8-a431-e1273a6d06f2@intel.com> (raw)
In-Reply-To: <20230803130337.3784597-4-suraj.kandpal@intel.com>
LGTM.
Perhaps we can have shorter name for the helper and variable.
something like intel_dsc_get_vdsc_per_pipe, vdsc_instanances_per_pipe?
With that fixed, this is:
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
On 8/3/2023 6:33 PM, Suraj Kandpal wrote:
> We have a function that gets us the total of the vdsc engines being
> used but not the no. of vdsc instances being used by each pipe.
>
> --v6
> -Change function to static
>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_vdsc.c | 78 +++++++++++++----------
> 1 file changed, 44 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 4437ade8fa87..cd7b7b00be44 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -350,9 +350,14 @@ intel_dsc_power_domain(struct intel_crtc *crtc, enum transcoder cpu_transcoder)
> return POWER_DOMAIN_TRANSCODER_VDSC_PW2;
> }
>
> +static int intel_dsc_get_no_vdsc_inst_per_pipe(const struct intel_crtc_state *crtc_state)
> +{
> + return crtc_state->dsc.dsc_split ? 2 : 1;
> +}
> +
> int intel_dsc_get_num_vdsc_instances(const struct intel_crtc_state *crtc_state)
> {
> - int num_vdsc_instances = (crtc_state->dsc.dsc_split) ? 2 : 1;
> + int num_vdsc_instances = intel_dsc_get_no_vdsc_inst_per_pipe(crtc_state);
>
> if (crtc_state->bigjoiner_pipes)
> num_vdsc_instances *= 2;
> @@ -372,6 +377,7 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> u32 rc_range_params_dword[8];
> int i = 0;
> int num_vdsc_instances = intel_dsc_get_num_vdsc_instances(crtc_state);
> + int no_vdsc_instances_per_pipe = intel_dsc_get_no_vdsc_inst_per_pipe(crtc_state);
>
> /* Populate PICTURE_PARAMETER_SET_0 registers */
> pps_val = DSC_VER_MAJ | vdsc_cfg->dsc_version_minor <<
> @@ -401,14 +407,14 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> * If 2 VDSC instances are needed, configure PPS for second
> * VDSC
> */
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv, DSCC_PICTURE_PARAMETER_SET_0,
> pps_val);
> } else {
> intel_de_write(dev_priv,
> ICL_DSC0_PICTURE_PARAMETER_SET_0(pipe),
> pps_val);
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> ICL_DSC1_PICTURE_PARAMETER_SET_0(pipe),
> pps_val);
> @@ -425,14 +431,14 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> * If 2 VDSC instances are needed, configure PPS for second
> * VDSC
> */
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv, DSCC_PICTURE_PARAMETER_SET_1,
> pps_val);
> } else {
> intel_de_write(dev_priv,
> ICL_DSC0_PICTURE_PARAMETER_SET_1(pipe),
> pps_val);
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> ICL_DSC1_PICTURE_PARAMETER_SET_1(pipe),
> pps_val);
> @@ -450,14 +456,14 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> * If 2 VDSC instances are needed, configure PPS for second
> * VDSC
> */
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv, DSCC_PICTURE_PARAMETER_SET_2,
> pps_val);
> } else {
> intel_de_write(dev_priv,
> ICL_DSC0_PICTURE_PARAMETER_SET_2(pipe),
> pps_val);
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> ICL_DSC1_PICTURE_PARAMETER_SET_2(pipe),
> pps_val);
> @@ -475,14 +481,14 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> * If 2 VDSC instances are needed, configure PPS for second
> * VDSC
> */
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv, DSCC_PICTURE_PARAMETER_SET_3,
> pps_val);
> } else {
> intel_de_write(dev_priv,
> ICL_DSC0_PICTURE_PARAMETER_SET_3(pipe),
> pps_val);
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> ICL_DSC1_PICTURE_PARAMETER_SET_3(pipe),
> pps_val);
> @@ -500,14 +506,14 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> * If 2 VDSC instances are needed, configure PPS for second
> * VDSC
> */
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv, DSCC_PICTURE_PARAMETER_SET_4,
> pps_val);
> } else {
> intel_de_write(dev_priv,
> ICL_DSC0_PICTURE_PARAMETER_SET_4(pipe),
> pps_val);
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> ICL_DSC1_PICTURE_PARAMETER_SET_4(pipe),
> pps_val);
> @@ -525,14 +531,14 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> * If 2 VDSC instances are needed, configure PPS for second
> * VDSC
> */
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv, DSCC_PICTURE_PARAMETER_SET_5,
> pps_val);
> } else {
> intel_de_write(dev_priv,
> ICL_DSC0_PICTURE_PARAMETER_SET_5(pipe),
> pps_val);
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> ICL_DSC1_PICTURE_PARAMETER_SET_5(pipe),
> pps_val);
> @@ -552,14 +558,14 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> * If 2 VDSC instances are needed, configure PPS for second
> * VDSC
> */
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv, DSCC_PICTURE_PARAMETER_SET_6,
> pps_val);
> } else {
> intel_de_write(dev_priv,
> ICL_DSC0_PICTURE_PARAMETER_SET_6(pipe),
> pps_val);
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> ICL_DSC1_PICTURE_PARAMETER_SET_6(pipe),
> pps_val);
> @@ -577,14 +583,14 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> * If 2 VDSC instances are needed, configure PPS for second
> * VDSC
> */
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv, DSCC_PICTURE_PARAMETER_SET_7,
> pps_val);
> } else {
> intel_de_write(dev_priv,
> ICL_DSC0_PICTURE_PARAMETER_SET_7(pipe),
> pps_val);
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> ICL_DSC1_PICTURE_PARAMETER_SET_7(pipe),
> pps_val);
> @@ -602,14 +608,14 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> * If 2 VDSC instances are needed, configure PPS for second
> * VDSC
> */
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv, DSCC_PICTURE_PARAMETER_SET_8,
> pps_val);
> } else {
> intel_de_write(dev_priv,
> ICL_DSC0_PICTURE_PARAMETER_SET_8(pipe),
> pps_val);
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> ICL_DSC1_PICTURE_PARAMETER_SET_8(pipe),
> pps_val);
> @@ -627,14 +633,14 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> * If 2 VDSC instances are needed, configure PPS for second
> * VDSC
> */
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv, DSCC_PICTURE_PARAMETER_SET_9,
> pps_val);
> } else {
> intel_de_write(dev_priv,
> ICL_DSC0_PICTURE_PARAMETER_SET_9(pipe),
> pps_val);
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> ICL_DSC1_PICTURE_PARAMETER_SET_9(pipe),
> pps_val);
> @@ -654,14 +660,14 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> * If 2 VDSC instances are needed, configure PPS for second
> * VDSC
> */
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> DSCC_PICTURE_PARAMETER_SET_10, pps_val);
> } else {
> intel_de_write(dev_priv,
> ICL_DSC0_PICTURE_PARAMETER_SET_10(pipe),
> pps_val);
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> ICL_DSC1_PICTURE_PARAMETER_SET_10(pipe),
> pps_val);
> @@ -682,14 +688,14 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> * If 2 VDSC instances are needed, configure PPS for second
> * VDSC
> */
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> DSCC_PICTURE_PARAMETER_SET_16, pps_val);
> } else {
> intel_de_write(dev_priv,
> ICL_DSC0_PICTURE_PARAMETER_SET_16(pipe),
> pps_val);
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> ICL_DSC1_PICTURE_PARAMETER_SET_16(pipe),
> pps_val);
> @@ -703,7 +709,7 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> intel_de_write(dev_priv,
> MTL_DSC0_PICTURE_PARAMETER_SET_17(pipe),
> pps_val);
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> MTL_DSC1_PICTURE_PARAMETER_SET_17(pipe),
> pps_val);
> @@ -716,7 +722,7 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> intel_de_write(dev_priv,
> MTL_DSC0_PICTURE_PARAMETER_SET_18(pipe),
> pps_val);
> - if (crtc_state->dsc.dsc_split)
> + if (no_vdsc_instances_per_pipe > 1)
> intel_de_write(dev_priv,
> MTL_DSC1_PICTURE_PARAMETER_SET_18(pipe),
> pps_val);
> @@ -740,7 +746,7 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> rc_buf_thresh_dword[2]);
> intel_de_write(dev_priv, DSCA_RC_BUF_THRESH_1_UDW,
> rc_buf_thresh_dword[3]);
> - if (crtc_state->dsc.dsc_split) {
> + if (no_vdsc_instances_per_pipe > 1) {
> intel_de_write(dev_priv, DSCC_RC_BUF_THRESH_0,
> rc_buf_thresh_dword[0]);
> intel_de_write(dev_priv, DSCC_RC_BUF_THRESH_0_UDW,
> @@ -759,7 +765,7 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> rc_buf_thresh_dword[2]);
> intel_de_write(dev_priv, ICL_DSC0_RC_BUF_THRESH_1_UDW(pipe),
> rc_buf_thresh_dword[3]);
> - if (crtc_state->dsc.dsc_split) {
> + if (no_vdsc_instances_per_pipe > 1) {
> intel_de_write(dev_priv,
> ICL_DSC1_RC_BUF_THRESH_0(pipe),
> rc_buf_thresh_dword[0]);
> @@ -805,7 +811,7 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> rc_range_params_dword[6]);
> intel_de_write(dev_priv, DSCA_RC_RANGE_PARAMETERS_3_UDW,
> rc_range_params_dword[7]);
> - if (crtc_state->dsc.dsc_split) {
> + if (no_vdsc_instances_per_pipe > 1) {
> intel_de_write(dev_priv, DSCC_RC_RANGE_PARAMETERS_0,
> rc_range_params_dword[0]);
> intel_de_write(dev_priv,
> @@ -848,7 +854,7 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
> intel_de_write(dev_priv,
> ICL_DSC0_RC_RANGE_PARAMETERS_3_UDW(pipe),
> rc_range_params_dword[7]);
> - if (crtc_state->dsc.dsc_split) {
> + if (no_vdsc_instances_per_pipe > 1) {
> intel_de_write(dev_priv,
> ICL_DSC1_RC_RANGE_PARAMETERS_0(pipe),
> rc_range_params_dword[0]);
> @@ -954,6 +960,7 @@ void intel_dsc_enable(const struct intel_crtc_state *crtc_state)
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> u32 dss_ctl1_val = 0;
> u32 dss_ctl2_val = 0;
> + int no_vdsc_instances_per_pipe = intel_dsc_get_no_vdsc_inst_per_pipe(crtc_state);
>
> if (!crtc_state->dsc.compression_enable)
> return;
> @@ -961,7 +968,7 @@ void intel_dsc_enable(const struct intel_crtc_state *crtc_state)
> intel_dsc_pps_configure(crtc_state);
>
> dss_ctl2_val |= LEFT_BRANCH_VDSC_ENABLE;
> - if (crtc_state->dsc.dsc_split) {
> + if (no_vdsc_instances_per_pipe > 1) {
> dss_ctl2_val |= RIGHT_BRANCH_VDSC_ENABLE;
> dss_ctl1_val |= JOINER_ENABLE;
> }
> @@ -997,6 +1004,7 @@ void intel_dsc_get_config(struct intel_crtc_state *crtc_state)
> enum intel_display_power_domain power_domain;
> intel_wakeref_t wakeref;
> u32 dss_ctl1, dss_ctl2, pps0 = 0, pps1 = 0, pps_temp0, pps_temp1;
> + int no_vdsc_instances_per_pipe;
>
> if (!intel_dsc_source_support(crtc_state))
> return;
> @@ -1019,10 +1027,12 @@ void intel_dsc_get_config(struct intel_crtc_state *crtc_state)
>
> /* FIXME: add more state readout as needed */
>
> + no_vdsc_instances_per_pipe = intel_dsc_get_no_vdsc_inst_per_pipe(crtc_state);
> +
> /* PPS0 & PPS1 */
> if (!is_pipe_dsc(crtc, cpu_transcoder)) {
> pps1 = intel_de_read(dev_priv, DSCA_PICTURE_PARAMETER_SET_1);
> - if (crtc_state->dsc.dsc_split) {
> + if (no_vdsc_instances_per_pipe > 1) {
> pps_temp1 = intel_de_read(dev_priv, DSCC_PICTURE_PARAMETER_SET_1);
> drm_WARN_ON(&dev_priv->drm, pps1 != pps_temp1);
> }
> @@ -1031,7 +1041,7 @@ void intel_dsc_get_config(struct intel_crtc_state *crtc_state)
> ICL_DSC0_PICTURE_PARAMETER_SET_0(pipe));
> pps1 = intel_de_read(dev_priv,
> ICL_DSC0_PICTURE_PARAMETER_SET_1(pipe));
> - if (crtc_state->dsc.dsc_split) {
> + if (no_vdsc_instances_per_pipe > 1) {
> pps_temp0 = intel_de_read(dev_priv,
> ICL_DSC1_PICTURE_PARAMETER_SET_0(pipe));
> pps_temp1 = intel_de_read(dev_priv,
next prev parent reply other threads:[~2023-08-08 8:09 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-03 13:03 [Intel-gfx] [PATCH v6 0/8] Add DSC PPS readout Suraj Kandpal
2023-08-03 13:03 ` [Intel-gfx] [PATCH v6 1/8] drm/i915/vdsc: Refactor dsc register field macro Suraj Kandpal
2023-08-03 13:03 ` [Intel-gfx] [PATCH v6 2/8] drm/i915/vdsc: Add a check for dsc split cases Suraj Kandpal
2023-08-03 13:03 ` [Intel-gfx] [PATCH v6 3/8] drm/i915/vdsc: Add func to get no. of vdsc instances per pipe Suraj Kandpal
2023-08-08 8:09 ` Nautiyal, Ankit K [this message]
2023-08-03 13:03 ` [Intel-gfx] [PATCH v6 4/8] drm/i915/vdsc: Add function to read any PPS register Suraj Kandpal
2023-08-07 13:20 ` Nautiyal, Ankit K
2023-08-07 14:03 ` Kandpal, Suraj
2023-08-08 7:48 ` Nautiyal, Ankit K
2023-08-03 13:03 ` [Intel-gfx] [PATCH v6 5/8] drm/i915/vdsc: Add function to write in " Suraj Kandpal
2023-08-07 13:26 ` Nautiyal, Ankit K
2023-08-03 13:03 ` [Intel-gfx] [PATCH v6 6/8] drm/i915/vdsc: Remove unused dsc registers Suraj Kandpal
2023-08-03 13:03 ` [Intel-gfx] [PATCH v6 7/8] drm/i915/vdsc: Fill the intel_dsc_get_pps_config function Suraj Kandpal
2023-08-03 13:03 ` [Intel-gfx] [PATCH v6 8/8] drm/i915/display: Compare the readout dsc pps params Suraj Kandpal
2023-08-07 13:28 ` Nautiyal, Ankit K
2023-08-03 16:50 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Add DSC PPS readout (rev6) Patchwork
2023-08-03 16:50 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-08-03 17:11 ` [Intel-gfx] ✗ Fi.CI.BAT: 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=cdbb8362-cc65-bed8-a431-e1273a6d06f2@intel.com \
--to=ankit.k.nautiyal@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=suraj.kandpal@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