From: "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>
To: "Kandpal, Suraj" <suraj.kandpal@intel.com>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH v3 4/7] drm/i915/vdsc: Use MACRO to cleanup intel_dsc_get_pps_reg
Date: Tue, 18 Jul 2023 09:24:08 +0000 [thread overview]
Message-ID: <6476b61a-79a5-9f2b-0b59-48f950e20ce1@intel.com> (raw)
In-Reply-To: <20230717100931.2989980-5-suraj.kandpal@intel.com>
On 7/17/2023 3:39 PM, Suraj Kandpal wrote:
> Use a Macro to clean up intel_dsc_get_pps_reg so that we don't have
> to replicate so many if/else blocks.
>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_vdsc.c | 124 ++++++----------------
> 1 file changed, 32 insertions(+), 92 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 6d319f351a12..ed8fda431226 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -313,131 +313,71 @@ static void intel_dsc_get_pps_reg(struct intel_crtc_state *crtc_state, int pps,
>
> pipe_dsc = is_pipe_dsc(crtc, cpu_transcoder);
>
> +#define PRE_MTL_GET_DSC_REGISTER(pps, is_pipe_dsc, pipe) do { \
> + if (is_pipe_dsc) { \
> + *dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_##pps(pipe); \
> + *dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_##pps(pipe); \
> + } else { \
> + *dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_##pps; \
> + *dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_##pps; \
I think it will be more efficient to add an enum intel_dsc_engine and
have a function that returns the reg for given, pps, engine and pipe.
something like:
static i915_reg_t intel_dsc_get_pps_reg(const struct intel_crtc_state
*crtc_state, int pps, enum vdsc_engine vdsc)
Since offsets are regularly placed, we can have something like :
ICL_DSC_PICTURE_PARAMETER_SET(engine, pps, pipe) and
DSC_PICTURE_PARAMETER_SET(engine, pps) for ICL EDP/DSI case.
Roughly it will be Offset of PPS Reg for Engine 0 + engine * 0x800 + pps
* 4.
Though this will require quite some changes in reg macros in
intel_vdsc_reg.h ,and also need to have a new enum for intel_dsc_engine,
but IMHO it will be easier to get the required PPS reg this way.
Regards,
Ankit
> + } \
> +} while (0)
> +
> +#define MTL_GET_DSC_REGISTER(pps, pipe) do { \
> + *dsc_reg0 = MTL_DSC0_PICTURE_PARAMETER_SET_##pps(pipe); \
> + *dsc_reg1 = MTL_DSC1_PICTURE_PARAMETER_SET_##pps(pipe); \
> +} while (0)
> +
> switch (pps) {
> case 0:
> - if (pipe_dsc) {
> - *dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_0(pipe);
> - *dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_0(pipe);
> - } else {
> - *dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_0;
> - *dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_0;
> - }
> + PRE_MTL_GET_DSC_REGISTER(0, pipe_dsc, pipe);
> break;
> case 1:
> - if (pipe_dsc) {
> - *dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_1(pipe);
> - *dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_1(pipe);
> - } else {
> - *dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_1;
> - *dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_1;
> - }
> + PRE_MTL_GET_DSC_REGISTER(1, pipe_dsc, pipe);
> break;
> case 2:
> - if (pipe_dsc) {
> - *dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_2(pipe);
> - *dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_2(pipe);
> - } else {
> - *dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_2;
> - *dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_2;
> - }
> + PRE_MTL_GET_DSC_REGISTER(2, pipe_dsc, pipe);
> break;
> case 3:
> - if (pipe_dsc) {
> - *dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_3(pipe);
> - *dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_3(pipe);
> - } else {
> - *dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_3;
> - *dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_3;
> - }
> + PRE_MTL_GET_DSC_REGISTER(3, pipe_dsc, pipe);
> break;
> case 4:
> - if (pipe_dsc) {
> - *dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_4(pipe);
> - *dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_4(pipe);
> - } else {
> - *dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_4;
> - *dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_4;
> - }
> + PRE_MTL_GET_DSC_REGISTER(4, pipe_dsc, pipe);
> break;
> case 5:
> - if (pipe_dsc) {
> - *dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_5(pipe);
> - *dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_5(pipe);
> - } else {
> - *dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_5;
> - *dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_5;
> - }
> + PRE_MTL_GET_DSC_REGISTER(5, pipe_dsc, pipe);
> break;
> case 6:
> - if (pipe_dsc) {
> - *dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_6(pipe);
> - *dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_6(pipe);
> - } else {
> - *dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_6;
> - *dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_6;
> - }
> + PRE_MTL_GET_DSC_REGISTER(6, pipe_dsc, pipe);
> break;
> case 7:
> - if (pipe_dsc) {
> - *dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_7(pipe);
> - *dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_7(pipe);
> - } else {
> - *dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_7;
> - *dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_7;
> - }
> + PRE_MTL_GET_DSC_REGISTER(7, pipe_dsc, pipe);
> break;
> case 8:
> - if (pipe_dsc) {
> - *dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_8(pipe);
> - *dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_8(pipe);
> - } else {
> - *dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_8;
> - *dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_8;
> - }
> + PRE_MTL_GET_DSC_REGISTER(8, pipe_dsc, pipe);
> break;
> case 9:
> - if (pipe_dsc) {
> - *dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_9(pipe);
> - *dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_9(pipe);
> - } else {
> - *dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_9;
> - *dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_9;
> - }
> + PRE_MTL_GET_DSC_REGISTER(9, pipe_dsc, pipe);
> break;
> case 10:
> - if (pipe_dsc) {
> - *dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_10(pipe);
> - *dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_10(pipe);
> - } else {
> - *dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_10;
> - *dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_10;
> - }
> + PRE_MTL_GET_DSC_REGISTER(10, pipe_dsc, pipe);
> break;
> case 16:
> - if (pipe_dsc) {
> - *dsc_reg0 = ICL_DSC0_PICTURE_PARAMETER_SET_16(pipe);
> - *dsc_reg1 = ICL_DSC1_PICTURE_PARAMETER_SET_16(pipe);
> - } else {
> - *dsc_reg0 = DSCA_PICTURE_PARAMETER_SET_16;
> - *dsc_reg1 = DSCC_PICTURE_PARAMETER_SET_16;
> - }
> + PRE_MTL_GET_DSC_REGISTER(16, pipe_dsc, pipe);
> break;
> - /*
> - * Since PPS_17 and PPS_18 were introduced from MTL dsc check
> - * need not be done
> - */
> case 17:
> - *dsc_reg0 = MTL_DSC0_PICTURE_PARAMETER_SET_17(pipe);
> - *dsc_reg1 = MTL_DSC1_PICTURE_PARAMETER_SET_17(pipe);
> + MTL_GET_DSC_REGISTER(17, pipe);
> break;
> case 18:
> - *dsc_reg0 = MTL_DSC0_PICTURE_PARAMETER_SET_18(pipe);
> - *dsc_reg1 = MTL_DSC1_PICTURE_PARAMETER_SET_18(pipe);
> + MTL_GET_DSC_REGISTER(18, pipe);
> break;
> default:
> MISSING_CASE(pps);
> break;
> }
> +
> +#undef PRE_MTL_GET_DSC_REGISTER
> +#undef MTL_GET_DSC_REGISTER
> }
>
> static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
next prev parent reply other threads:[~2023-07-18 9:24 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-17 10:09 [Intel-gfx] [PATCH v3 0/7] Add DSC PPS readout Suraj Kandpal
2023-07-17 10:09 ` [Intel-gfx] [PATCH v3 1/7] drm/i915/vdsc: Refactor dsc register field macro Suraj Kandpal
2023-07-17 10:09 ` [Intel-gfx] [PATCH v3 2/7] drm/i915/vdsc: Add a check for dsc split cases Suraj Kandpal
2023-07-17 10:09 ` [Intel-gfx] [PATCH v3 3/7] drm/i915/vdsc: Add function to read any PPS register Suraj Kandpal
2023-07-17 10:09 ` [Intel-gfx] [PATCH v3 4/7] drm/i915/vdsc: Use MACRO to cleanup intel_dsc_get_pps_reg Suraj Kandpal
2023-07-18 9:24 ` Nautiyal, Ankit K [this message]
2023-07-18 9:43 ` Nautiyal, Ankit K
2023-07-17 10:09 ` [Intel-gfx] [PATCH v3 5/7] drm/i915/vdsc: Add function to write in PPS register Suraj Kandpal
2023-07-17 10:09 ` [Intel-gfx] [PATCH v3 6/7] drm/i915/vdsc: Fill the intel_dsc_get_pps_config function Suraj Kandpal
2023-07-17 10:09 ` [Intel-gfx] [PATCH v3 7/7] drm/i915/display: Compare the readout dsc pps params Suraj Kandpal
2023-07-17 14:48 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Add DSC PPS readout (rev3) Patchwork
2023-07-17 14:48 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-07-17 15:07 ` [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=6476b61a-79a5-9f2b-0b59-48f950e20ce1@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