| Message ID | e2551b52ac0dd2b4ffe18d5e7733fafdc191d68a.1693933849.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/dsc: cleanups | expand |
> Subject: [PATCH 1/8] drm/i915/dsc: improve clarify of the pps reg read/write > helpers Should be clarity here in the commit header With that fixed Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com> > > Make it clear what's the number of vdsc per pipe, and what's the number of > registers to grab. Have intel_dsc_get_pps_reg() return the registers it knows > even if the requested amount is bigger. > > Cc: Suraj Kandpal <suraj.kandpal@intel.com> > Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/display/intel_vdsc.c | 40 ++++++++++++----------- > 1 file changed, 21 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c > b/drivers/gpu/drm/i915/display/intel_vdsc.c > index b24601d0b2c5..14317bb6d3df 100644 > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c > @@ -372,7 +372,7 @@ int intel_dsc_get_num_vdsc_instances(const struct > intel_crtc_state *crtc_state) } > > static void intel_dsc_get_pps_reg(const struct intel_crtc_state *crtc_state, int > pps, > - i915_reg_t *dsc_reg, int vdsc_per_pipe) > + i915_reg_t *dsc_reg, int dsc_reg_num) > { > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; @@ - > 381,16 +381,12 @@ static void intel_dsc_get_pps_reg(const struct > intel_crtc_state *crtc_state, int > > pipe_dsc = is_pipe_dsc(crtc, cpu_transcoder); > > - switch (vdsc_per_pipe) { > - case 2: > + if (dsc_reg_num >= 3) > + MISSING_CASE(dsc_reg_num); > + if (dsc_reg_num >= 2) > dsc_reg[1] = pipe_dsc ? ICL_DSC1_PPS(pipe, pps) : > DSCC_PPS(pps); > - fallthrough; > - case 1: > + if (dsc_reg_num >= 1) > dsc_reg[0] = pipe_dsc ? ICL_DSC0_PPS(pipe, pps) : > DSCA_PPS(pps); > - break; > - default: > - MISSING_CASE(vdsc_per_pipe); > - } > } > > static void intel_dsc_write_pps_reg(const struct intel_crtc_state *crtc_state, > @@ -399,13 +395,16 @@ static void intel_dsc_write_pps_reg(const struct > intel_crtc_state *crtc_state, > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > struct drm_i915_private *i915 = to_i915(crtc->base.dev); > i915_reg_t dsc_reg[2]; > - int i, vdsc_per_pipe = intel_dsc_get_vdsc_per_pipe(crtc_state); > + int i, vdsc_per_pipe, dsc_reg_num; > + > + vdsc_per_pipe = intel_dsc_get_vdsc_per_pipe(crtc_state); > + dsc_reg_num = min_t(int, ARRAY_SIZE(dsc_reg), vdsc_per_pipe); > > - drm_WARN_ON_ONCE(&i915->drm, ARRAY_SIZE(dsc_reg) < > vdsc_per_pipe); > + drm_WARN_ON_ONCE(&i915->drm, dsc_reg_num < vdsc_per_pipe); > > - intel_dsc_get_pps_reg(crtc_state, pps, dsc_reg, vdsc_per_pipe); > + intel_dsc_get_pps_reg(crtc_state, pps, dsc_reg, dsc_reg_num); > > - for (i = 0; i < min_t(int, ARRAY_SIZE(dsc_reg), vdsc_per_pipe); i++) > + for (i = 0; i < dsc_reg_num; i++) > intel_de_write(i915, dsc_reg[i], pps_val); } > > @@ -815,16 +814,19 @@ static bool intel_dsc_read_pps_reg(struct > intel_crtc_state *crtc_state, { > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > struct drm_i915_private *i915 = to_i915(crtc->base.dev); > - const int vdsc_per_pipe = intel_dsc_get_vdsc_per_pipe(crtc_state); > i915_reg_t dsc_reg[2]; > - int i; > + int i, vdsc_per_pipe, dsc_reg_num; > > - *pps_val = 0; > - drm_WARN_ON_ONCE(&i915->drm, ARRAY_SIZE(dsc_reg) < > vdsc_per_pipe); > + vdsc_per_pipe = intel_dsc_get_vdsc_per_pipe(crtc_state); > + dsc_reg_num = min_t(int, ARRAY_SIZE(dsc_reg), vdsc_per_pipe); > > - intel_dsc_get_pps_reg(crtc_state, pps, dsc_reg, vdsc_per_pipe); > + drm_WARN_ON_ONCE(&i915->drm, dsc_reg_num < vdsc_per_pipe); > + > + intel_dsc_get_pps_reg(crtc_state, pps, dsc_reg, dsc_reg_num); > + > + *pps_val = 0; > > - for (i = 0; i < min_t(int, ARRAY_SIZE(dsc_reg), vdsc_per_pipe); i++) { > + for (i = 0; i < dsc_reg_num; i++) { > u32 pps_temp; > > pps_temp = intel_de_read(i915, dsc_reg[i]); > -- > 2.39.2
On Thu, 07 Sep 2023, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote: >> Subject: [PATCH 1/8] drm/i915/dsc: improve clarify of the pps reg read/write >> helpers > > Should be clarity here in the commit header Thanks, fixed. > > With that fixed > Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com> Thanks for the reviews, pushed the lot to drm-intel-next. BR, Jani. >> >> Make it clear what's the number of vdsc per pipe, and what's the number of >> registers to grab. Have intel_dsc_get_pps_reg() return the registers it knows >> even if the requested amount is bigger. >> >> Cc: Suraj Kandpal <suraj.kandpal@intel.com> >> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_vdsc.c | 40 ++++++++++++----------- >> 1 file changed, 21 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c >> b/drivers/gpu/drm/i915/display/intel_vdsc.c >> index b24601d0b2c5..14317bb6d3df 100644 >> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c >> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c >> @@ -372,7 +372,7 @@ int intel_dsc_get_num_vdsc_instances(const struct >> intel_crtc_state *crtc_state) } >> >> static void intel_dsc_get_pps_reg(const struct intel_crtc_state *crtc_state, int >> pps, >> - i915_reg_t *dsc_reg, int vdsc_per_pipe) >> + i915_reg_t *dsc_reg, int dsc_reg_num) >> { >> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); >> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; @@ - >> 381,16 +381,12 @@ static void intel_dsc_get_pps_reg(const struct >> intel_crtc_state *crtc_state, int >> >> pipe_dsc = is_pipe_dsc(crtc, cpu_transcoder); >> >> - switch (vdsc_per_pipe) { >> - case 2: >> + if (dsc_reg_num >= 3) >> + MISSING_CASE(dsc_reg_num); >> + if (dsc_reg_num >= 2) >> dsc_reg[1] = pipe_dsc ? ICL_DSC1_PPS(pipe, pps) : >> DSCC_PPS(pps); >> - fallthrough; >> - case 1: >> + if (dsc_reg_num >= 1) >> dsc_reg[0] = pipe_dsc ? ICL_DSC0_PPS(pipe, pps) : >> DSCA_PPS(pps); >> - break; >> - default: >> - MISSING_CASE(vdsc_per_pipe); >> - } >> } >> >> static void intel_dsc_write_pps_reg(const struct intel_crtc_state *crtc_state, >> @@ -399,13 +395,16 @@ static void intel_dsc_write_pps_reg(const struct >> intel_crtc_state *crtc_state, >> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); >> struct drm_i915_private *i915 = to_i915(crtc->base.dev); >> i915_reg_t dsc_reg[2]; >> - int i, vdsc_per_pipe = intel_dsc_get_vdsc_per_pipe(crtc_state); >> + int i, vdsc_per_pipe, dsc_reg_num; >> + >> + vdsc_per_pipe = intel_dsc_get_vdsc_per_pipe(crtc_state); >> + dsc_reg_num = min_t(int, ARRAY_SIZE(dsc_reg), vdsc_per_pipe); >> >> - drm_WARN_ON_ONCE(&i915->drm, ARRAY_SIZE(dsc_reg) < >> vdsc_per_pipe); >> + drm_WARN_ON_ONCE(&i915->drm, dsc_reg_num < vdsc_per_pipe); >> >> - intel_dsc_get_pps_reg(crtc_state, pps, dsc_reg, vdsc_per_pipe); >> + intel_dsc_get_pps_reg(crtc_state, pps, dsc_reg, dsc_reg_num); >> >> - for (i = 0; i < min_t(int, ARRAY_SIZE(dsc_reg), vdsc_per_pipe); i++) >> + for (i = 0; i < dsc_reg_num; i++) >> intel_de_write(i915, dsc_reg[i], pps_val); } >> >> @@ -815,16 +814,19 @@ static bool intel_dsc_read_pps_reg(struct >> intel_crtc_state *crtc_state, { >> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); >> struct drm_i915_private *i915 = to_i915(crtc->base.dev); >> - const int vdsc_per_pipe = intel_dsc_get_vdsc_per_pipe(crtc_state); >> i915_reg_t dsc_reg[2]; >> - int i; >> + int i, vdsc_per_pipe, dsc_reg_num; >> >> - *pps_val = 0; >> - drm_WARN_ON_ONCE(&i915->drm, ARRAY_SIZE(dsc_reg) < >> vdsc_per_pipe); >> + vdsc_per_pipe = intel_dsc_get_vdsc_per_pipe(crtc_state); >> + dsc_reg_num = min_t(int, ARRAY_SIZE(dsc_reg), vdsc_per_pipe); >> >> - intel_dsc_get_pps_reg(crtc_state, pps, dsc_reg, vdsc_per_pipe); >> + drm_WARN_ON_ONCE(&i915->drm, dsc_reg_num < vdsc_per_pipe); >> + >> + intel_dsc_get_pps_reg(crtc_state, pps, dsc_reg, dsc_reg_num); >> + >> + *pps_val = 0; >> >> - for (i = 0; i < min_t(int, ARRAY_SIZE(dsc_reg), vdsc_per_pipe); i++) { >> + for (i = 0; i < dsc_reg_num; i++) { >> u32 pps_temp; >> >> pps_temp = intel_de_read(i915, dsc_reg[i]); >> -- >> 2.39.2 >
diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c index b24601d0b2c5..14317bb6d3df 100644 --- a/drivers/gpu/drm/i915/display/intel_vdsc.c +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c @@ -372,7 +372,7 @@ int intel_dsc_get_num_vdsc_instances(const struct intel_crtc_state *crtc_state) } static void intel_dsc_get_pps_reg(const struct intel_crtc_state *crtc_state, int pps, - i915_reg_t *dsc_reg, int vdsc_per_pipe) + i915_reg_t *dsc_reg, int dsc_reg_num) { struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; @@ -381,16 +381,12 @@ static void intel_dsc_get_pps_reg(const struct intel_crtc_state *crtc_state, int pipe_dsc = is_pipe_dsc(crtc, cpu_transcoder); - switch (vdsc_per_pipe) { - case 2: + if (dsc_reg_num >= 3) + MISSING_CASE(dsc_reg_num); + if (dsc_reg_num >= 2) dsc_reg[1] = pipe_dsc ? ICL_DSC1_PPS(pipe, pps) : DSCC_PPS(pps); - fallthrough; - case 1: + if (dsc_reg_num >= 1) dsc_reg[0] = pipe_dsc ? ICL_DSC0_PPS(pipe, pps) : DSCA_PPS(pps); - break; - default: - MISSING_CASE(vdsc_per_pipe); - } } static void intel_dsc_write_pps_reg(const struct intel_crtc_state *crtc_state, @@ -399,13 +395,16 @@ static void intel_dsc_write_pps_reg(const struct intel_crtc_state *crtc_state, struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); struct drm_i915_private *i915 = to_i915(crtc->base.dev); i915_reg_t dsc_reg[2]; - int i, vdsc_per_pipe = intel_dsc_get_vdsc_per_pipe(crtc_state); + int i, vdsc_per_pipe, dsc_reg_num; + + vdsc_per_pipe = intel_dsc_get_vdsc_per_pipe(crtc_state); + dsc_reg_num = min_t(int, ARRAY_SIZE(dsc_reg), vdsc_per_pipe); - drm_WARN_ON_ONCE(&i915->drm, ARRAY_SIZE(dsc_reg) < vdsc_per_pipe); + drm_WARN_ON_ONCE(&i915->drm, dsc_reg_num < vdsc_per_pipe); - intel_dsc_get_pps_reg(crtc_state, pps, dsc_reg, vdsc_per_pipe); + intel_dsc_get_pps_reg(crtc_state, pps, dsc_reg, dsc_reg_num); - for (i = 0; i < min_t(int, ARRAY_SIZE(dsc_reg), vdsc_per_pipe); i++) + for (i = 0; i < dsc_reg_num; i++) intel_de_write(i915, dsc_reg[i], pps_val); } @@ -815,16 +814,19 @@ static bool intel_dsc_read_pps_reg(struct intel_crtc_state *crtc_state, { struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); struct drm_i915_private *i915 = to_i915(crtc->base.dev); - const int vdsc_per_pipe = intel_dsc_get_vdsc_per_pipe(crtc_state); i915_reg_t dsc_reg[2]; - int i; + int i, vdsc_per_pipe, dsc_reg_num; - *pps_val = 0; - drm_WARN_ON_ONCE(&i915->drm, ARRAY_SIZE(dsc_reg) < vdsc_per_pipe); + vdsc_per_pipe = intel_dsc_get_vdsc_per_pipe(crtc_state); + dsc_reg_num = min_t(int, ARRAY_SIZE(dsc_reg), vdsc_per_pipe); - intel_dsc_get_pps_reg(crtc_state, pps, dsc_reg, vdsc_per_pipe); + drm_WARN_ON_ONCE(&i915->drm, dsc_reg_num < vdsc_per_pipe); + + intel_dsc_get_pps_reg(crtc_state, pps, dsc_reg, dsc_reg_num); + + *pps_val = 0; - for (i = 0; i < min_t(int, ARRAY_SIZE(dsc_reg), vdsc_per_pipe); i++) { + for (i = 0; i < dsc_reg_num; i++) { u32 pps_temp; pps_temp = intel_de_read(i915, dsc_reg[i]);
Make it clear what's the number of vdsc per pipe, and what's the number of registers to grab. Have intel_dsc_get_pps_reg() return the registers it knows even if the requested amount is bigger. Cc: Suraj Kandpal <suraj.kandpal@intel.com> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/display/intel_vdsc.c | 40 ++++++++++++----------- 1 file changed, 21 insertions(+), 19 deletions(-)