From: Manasi Navare <manasi.d.navare@intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2 3/4] drm/i915/dsc: split out encoder specific parts from DSC compute params
Date: Mon, 4 Nov 2019 14:41:21 -0800 [thread overview]
Message-ID: <20191104224121.GE32264@intel.com> (raw)
In-Reply-To: <20191104141439.26312-3-jani.nikula@intel.com>
On Mon, Nov 04, 2019 at 04:14:38PM +0200, Jani Nikula wrote:
> Split out the DP specific parts, making it easier to add DSI specific
> configuration. Also move the encoder specific parts towards the end, to
> allow overriding generic configuration if needed. This also improves
> clarity by making it clear the encoder independent configuration does
> not depend on the encoder specific parts.
>
> v2: Rebase
>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
The splitting looks good and def more organized in terms of DPCD vs pipe config DSC params
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
> drivers/gpu/drm/i915/display/intel_vdsc.c | 68 ++++++++++++++---------
> drivers/gpu/drm/i915/display/intel_vdsc.h | 5 +-
> 3 files changed, 45 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index fe1d683eab28..9e9593965a9a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2132,7 +2132,7 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> }
> }
>
> - ret = intel_dp_compute_dsc_params(intel_dp, pipe_config);
> + ret = intel_dsc_compute_params(&dig_port->base, pipe_config);
> if (ret < 0) {
> DRM_DEBUG_KMS("Cannot compute valid DSC parameters for Input Bpp = %d "
> "Compressed BPP = %d\n",
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index f1df654369a7..ac10736a076a 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -335,32 +335,14 @@ static const struct rc_parameters *get_rc_params(u16 compressed_bpp,
> return &rc_parameters[row_index][column_index];
> }
>
> -int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
> - struct intel_crtc_state *pipe_config)
> +/* Values filled from DSC Sink DPCD */
> +static int intel_dsc_dp_compute_params(struct intel_encoder *encoder,
> + struct intel_crtc_state *pipe_config)
> {
> + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> struct drm_dsc_config *vdsc_cfg = &pipe_config->dsc.config;
> - u16 compressed_bpp = pipe_config->dsc.compressed_bpp;
> - const struct rc_parameters *rc_params;
> - u8 i = 0;
> - u8 line_buf_depth = 0;
> + u8 line_buf_depth;
>
> - vdsc_cfg->pic_width = pipe_config->hw.adjusted_mode.crtc_hdisplay;
> - vdsc_cfg->pic_height = pipe_config->hw.adjusted_mode.crtc_vdisplay;
> - vdsc_cfg->slice_width = DIV_ROUND_UP(vdsc_cfg->pic_width,
> - pipe_config->dsc.slice_count);
> - /*
> - * Slice Height of 8 works for all currently available panels. So start
> - * with that if pic_height is an integral multiple of 8.
> - * Eventually add logic to try multiple slice heights.
> - */
> - if (vdsc_cfg->pic_height % 8 == 0)
> - vdsc_cfg->slice_height = 8;
> - else if (vdsc_cfg->pic_height % 4 == 0)
> - vdsc_cfg->slice_height = 4;
> - else
> - vdsc_cfg->slice_height = 2;
> -
> - /* Values filled from DSC Sink DPCD */
> vdsc_cfg->dsc_version_major =
> (intel_dp->dsc_dpcd[DP_DSC_REV - DP_DSC_SUPPORT] &
> DP_DSC_MAJOR_MASK) >> DP_DSC_MAJOR_SHIFT;
> @@ -377,6 +359,7 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
> DRM_DEBUG_KMS("DSC Sink Line Buffer Depth invalid\n");
> return -EINVAL;
> }
> +
> if (vdsc_cfg->dsc_version_minor == 2)
> vdsc_cfg->line_buf_depth = (line_buf_depth == DSC_1_2_MAX_LINEBUF_DEPTH_BITS) ?
> DSC_1_2_MAX_LINEBUF_DEPTH_VAL : line_buf_depth;
> @@ -384,13 +367,42 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
> vdsc_cfg->line_buf_depth = (line_buf_depth > DSC_1_1_MAX_LINEBUF_DEPTH_BITS) ?
> DSC_1_1_MAX_LINEBUF_DEPTH_BITS : line_buf_depth;
>
> + vdsc_cfg->block_pred_enable =
> + intel_dp->dsc_dpcd[DP_DSC_BLK_PREDICTION_SUPPORT - DP_DSC_SUPPORT] &
> + DP_DSC_BLK_PREDICTION_IS_SUPPORTED;
> +
> + return 0;
> +}
> +
> +int intel_dsc_compute_params(struct intel_encoder *encoder,
> + struct intel_crtc_state *pipe_config)
> +{
> + struct drm_dsc_config *vdsc_cfg = &pipe_config->dsc.config;
> + u16 compressed_bpp = pipe_config->dsc.compressed_bpp;
> + const struct rc_parameters *rc_params;
> + u8 i = 0;
> + int ret;
> +
> + vdsc_cfg->pic_width = pipe_config->hw.adjusted_mode.crtc_hdisplay;
> + vdsc_cfg->pic_height = pipe_config->hw.adjusted_mode.crtc_vdisplay;
> + vdsc_cfg->slice_width = DIV_ROUND_UP(vdsc_cfg->pic_width,
> + pipe_config->dsc.slice_count);
> + /*
> + * Slice Height of 8 works for all currently available panels. So start
> + * with that if pic_height is an integral multiple of 8.
> + * Eventually add logic to try multiple slice heights.
> + */
> + if (vdsc_cfg->pic_height % 8 == 0)
> + vdsc_cfg->slice_height = 8;
> + else if (vdsc_cfg->pic_height % 4 == 0)
> + vdsc_cfg->slice_height = 4;
> + else
> + vdsc_cfg->slice_height = 2;
> +
> /* Gen 11 does not support YCbCr */
> vdsc_cfg->simple_422 = false;
> /* Gen 11 does not support VBR */
> vdsc_cfg->vbr_enable = false;
> - vdsc_cfg->block_pred_enable =
> - intel_dp->dsc_dpcd[DP_DSC_BLK_PREDICTION_SUPPORT - DP_DSC_SUPPORT] &
> - DP_DSC_BLK_PREDICTION_IS_SUPPORTED;
>
> /* Gen 11 only supports integral values of bpp */
> vdsc_cfg->bits_per_pixel = compressed_bpp << 4;
> @@ -458,6 +470,10 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
> vdsc_cfg->initial_scale_value = (vdsc_cfg->rc_model_size << 3) /
> (vdsc_cfg->rc_model_size - vdsc_cfg->initial_offset);
>
> + ret = intel_dsc_dp_compute_params(encoder, pipe_config);
> + if (ret)
> + return ret;
> +
> return drm_dsc_compute_rc_parameters(vdsc_cfg);
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.h b/drivers/gpu/drm/i915/display/intel_vdsc.h
> index 90d3f6017fcb..4ed2256750c3 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.h
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.h
> @@ -8,13 +8,12 @@
>
> struct intel_encoder;
> struct intel_crtc_state;
> -struct intel_dp;
>
> void intel_dsc_enable(struct intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state);
> void intel_dsc_disable(const struct intel_crtc_state *crtc_state);
> -int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
> - struct intel_crtc_state *pipe_config);
> +int intel_dsc_compute_params(struct intel_encoder *encoder,
> + struct intel_crtc_state *pipe_config);
> enum intel_display_power_domain
> intel_dsc_power_domain(const struct intel_crtc_state *crtc_state);
>
> --
> 2.20.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
WARNING: multiple messages have this Message-ID (diff)
From: Manasi Navare <manasi.d.navare@intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 3/4] drm/i915/dsc: split out encoder specific parts from DSC compute params
Date: Mon, 4 Nov 2019 14:41:21 -0800 [thread overview]
Message-ID: <20191104224121.GE32264@intel.com> (raw)
Message-ID: <20191104224121.CUSrGhoMn7RwRRpEScun13cabO3YnyTJFrcZeelNPpU@z> (raw)
In-Reply-To: <20191104141439.26312-3-jani.nikula@intel.com>
On Mon, Nov 04, 2019 at 04:14:38PM +0200, Jani Nikula wrote:
> Split out the DP specific parts, making it easier to add DSI specific
> configuration. Also move the encoder specific parts towards the end, to
> allow overriding generic configuration if needed. This also improves
> clarity by making it clear the encoder independent configuration does
> not depend on the encoder specific parts.
>
> v2: Rebase
>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
The splitting looks good and def more organized in terms of DPCD vs pipe config DSC params
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
> drivers/gpu/drm/i915/display/intel_vdsc.c | 68 ++++++++++++++---------
> drivers/gpu/drm/i915/display/intel_vdsc.h | 5 +-
> 3 files changed, 45 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index fe1d683eab28..9e9593965a9a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2132,7 +2132,7 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> }
> }
>
> - ret = intel_dp_compute_dsc_params(intel_dp, pipe_config);
> + ret = intel_dsc_compute_params(&dig_port->base, pipe_config);
> if (ret < 0) {
> DRM_DEBUG_KMS("Cannot compute valid DSC parameters for Input Bpp = %d "
> "Compressed BPP = %d\n",
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index f1df654369a7..ac10736a076a 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -335,32 +335,14 @@ static const struct rc_parameters *get_rc_params(u16 compressed_bpp,
> return &rc_parameters[row_index][column_index];
> }
>
> -int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
> - struct intel_crtc_state *pipe_config)
> +/* Values filled from DSC Sink DPCD */
> +static int intel_dsc_dp_compute_params(struct intel_encoder *encoder,
> + struct intel_crtc_state *pipe_config)
> {
> + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> struct drm_dsc_config *vdsc_cfg = &pipe_config->dsc.config;
> - u16 compressed_bpp = pipe_config->dsc.compressed_bpp;
> - const struct rc_parameters *rc_params;
> - u8 i = 0;
> - u8 line_buf_depth = 0;
> + u8 line_buf_depth;
>
> - vdsc_cfg->pic_width = pipe_config->hw.adjusted_mode.crtc_hdisplay;
> - vdsc_cfg->pic_height = pipe_config->hw.adjusted_mode.crtc_vdisplay;
> - vdsc_cfg->slice_width = DIV_ROUND_UP(vdsc_cfg->pic_width,
> - pipe_config->dsc.slice_count);
> - /*
> - * Slice Height of 8 works for all currently available panels. So start
> - * with that if pic_height is an integral multiple of 8.
> - * Eventually add logic to try multiple slice heights.
> - */
> - if (vdsc_cfg->pic_height % 8 == 0)
> - vdsc_cfg->slice_height = 8;
> - else if (vdsc_cfg->pic_height % 4 == 0)
> - vdsc_cfg->slice_height = 4;
> - else
> - vdsc_cfg->slice_height = 2;
> -
> - /* Values filled from DSC Sink DPCD */
> vdsc_cfg->dsc_version_major =
> (intel_dp->dsc_dpcd[DP_DSC_REV - DP_DSC_SUPPORT] &
> DP_DSC_MAJOR_MASK) >> DP_DSC_MAJOR_SHIFT;
> @@ -377,6 +359,7 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
> DRM_DEBUG_KMS("DSC Sink Line Buffer Depth invalid\n");
> return -EINVAL;
> }
> +
> if (vdsc_cfg->dsc_version_minor == 2)
> vdsc_cfg->line_buf_depth = (line_buf_depth == DSC_1_2_MAX_LINEBUF_DEPTH_BITS) ?
> DSC_1_2_MAX_LINEBUF_DEPTH_VAL : line_buf_depth;
> @@ -384,13 +367,42 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
> vdsc_cfg->line_buf_depth = (line_buf_depth > DSC_1_1_MAX_LINEBUF_DEPTH_BITS) ?
> DSC_1_1_MAX_LINEBUF_DEPTH_BITS : line_buf_depth;
>
> + vdsc_cfg->block_pred_enable =
> + intel_dp->dsc_dpcd[DP_DSC_BLK_PREDICTION_SUPPORT - DP_DSC_SUPPORT] &
> + DP_DSC_BLK_PREDICTION_IS_SUPPORTED;
> +
> + return 0;
> +}
> +
> +int intel_dsc_compute_params(struct intel_encoder *encoder,
> + struct intel_crtc_state *pipe_config)
> +{
> + struct drm_dsc_config *vdsc_cfg = &pipe_config->dsc.config;
> + u16 compressed_bpp = pipe_config->dsc.compressed_bpp;
> + const struct rc_parameters *rc_params;
> + u8 i = 0;
> + int ret;
> +
> + vdsc_cfg->pic_width = pipe_config->hw.adjusted_mode.crtc_hdisplay;
> + vdsc_cfg->pic_height = pipe_config->hw.adjusted_mode.crtc_vdisplay;
> + vdsc_cfg->slice_width = DIV_ROUND_UP(vdsc_cfg->pic_width,
> + pipe_config->dsc.slice_count);
> + /*
> + * Slice Height of 8 works for all currently available panels. So start
> + * with that if pic_height is an integral multiple of 8.
> + * Eventually add logic to try multiple slice heights.
> + */
> + if (vdsc_cfg->pic_height % 8 == 0)
> + vdsc_cfg->slice_height = 8;
> + else if (vdsc_cfg->pic_height % 4 == 0)
> + vdsc_cfg->slice_height = 4;
> + else
> + vdsc_cfg->slice_height = 2;
> +
> /* Gen 11 does not support YCbCr */
> vdsc_cfg->simple_422 = false;
> /* Gen 11 does not support VBR */
> vdsc_cfg->vbr_enable = false;
> - vdsc_cfg->block_pred_enable =
> - intel_dp->dsc_dpcd[DP_DSC_BLK_PREDICTION_SUPPORT - DP_DSC_SUPPORT] &
> - DP_DSC_BLK_PREDICTION_IS_SUPPORTED;
>
> /* Gen 11 only supports integral values of bpp */
> vdsc_cfg->bits_per_pixel = compressed_bpp << 4;
> @@ -458,6 +470,10 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
> vdsc_cfg->initial_scale_value = (vdsc_cfg->rc_model_size << 3) /
> (vdsc_cfg->rc_model_size - vdsc_cfg->initial_offset);
>
> + ret = intel_dsc_dp_compute_params(encoder, pipe_config);
> + if (ret)
> + return ret;
> +
> return drm_dsc_compute_rc_parameters(vdsc_cfg);
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.h b/drivers/gpu/drm/i915/display/intel_vdsc.h
> index 90d3f6017fcb..4ed2256750c3 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.h
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.h
> @@ -8,13 +8,12 @@
>
> struct intel_encoder;
> struct intel_crtc_state;
> -struct intel_dp;
>
> void intel_dsc_enable(struct intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state);
> void intel_dsc_disable(const struct intel_crtc_state *crtc_state);
> -int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
> - struct intel_crtc_state *pipe_config);
> +int intel_dsc_compute_params(struct intel_encoder *encoder,
> + struct intel_crtc_state *pipe_config);
> enum intel_display_power_domain
> intel_dsc_power_domain(const struct intel_crtc_state *crtc_state);
>
> --
> 2.20.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-11-04 22:38 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-04 14:14 [PATCH v2 1/4] drm/i915/dsc: make parameter arrays const Jani Nikula
2019-11-04 14:14 ` [Intel-gfx] " Jani Nikula
2019-11-04 14:14 ` [PATCH v2 2/4] drm/i915/dsc: clean up rc parameter table access Jani Nikula
2019-11-04 14:14 ` [Intel-gfx] " Jani Nikula
2019-11-05 0:27 ` Manasi Navare
2019-11-05 0:27 ` [Intel-gfx] " Manasi Navare
2019-11-04 14:14 ` [PATCH v2 3/4] drm/i915/dsc: split out encoder specific parts from DSC compute params Jani Nikula
2019-11-04 14:14 ` [Intel-gfx] " Jani Nikula
2019-11-04 22:41 ` Manasi Navare [this message]
2019-11-04 22:41 ` Manasi Navare
2019-11-04 14:14 ` [PATCH v2 4/4] drm/i915/dsc: rename functions for consistency Jani Nikula
2019-11-04 14:14 ` [Intel-gfx] " Jani Nikula
2019-11-04 22:27 ` Manasi Navare
2019-11-04 22:27 ` [Intel-gfx] " Manasi Navare
2019-11-05 5:53 ` Jani Nikula
2019-11-05 5:53 ` [Intel-gfx] " Jani Nikula
2019-11-04 15:57 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/4] drm/i915/dsc: make parameter arrays const Patchwork
2019-11-04 15:57 ` [Intel-gfx] " Patchwork
2019-11-04 22:23 ` [PATCH v2 1/4] " Manasi Navare
2019-11-04 22:23 ` [Intel-gfx] " Manasi Navare
2019-11-05 13:14 ` Jani Nikula
2019-11-05 13:14 ` [Intel-gfx] " Jani Nikula
2019-11-05 11:32 ` ✓ Fi.CI.IGT: success for series starting with [v2,1/4] " Patchwork
2019-11-05 11:32 ` [Intel-gfx] " Patchwork
2019-11-05 13:25 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/4] drm/i915/dsc: make parameter arrays const (rev2) Patchwork
2019-11-05 13:25 ` [Intel-gfx] " Patchwork
2019-11-06 0:46 ` ✓ Fi.CI.IGT: " Patchwork
2019-11-06 0:46 ` [Intel-gfx] " 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=20191104224121.GE32264@intel.com \
--to=manasi.d.navare@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@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.