[PATCH v2 1/3] drm/i915/display: Use display variable in intel_bw.c
Vodapalli, Ravi Kumar
ravi.kumar.vodapalli at intel.com
Fri Feb 21 12:15:37 UTC 2025
On 2/18/2025 1:35 AM, Gustavo Sousa wrote:
> Quoting Ville Syrjälä (2025-02-17 14:44:39-03:00)
>> On Mon, Feb 17, 2025 at 12:35:23PM -0300, Gustavo Sousa wrote:
>>> Update intel_bw.c to use a "display" variable to refer to members of the
>>> display struct. While this change does not move that file to completely
>>> use struct intel_display as part of it's internal and public interface,
>>> this should help with a future transition.
>> Is there a particular reason you're not converting the whole
>> thing? I'd prefer to see a full conversion here.
> May original intent was patch #3, but then I thought I could help with
> the conversion with patches #1 and #2, since I would be using
> DISPLAY_VER()...
>
> I guess I was just lazy in not doing the full conversion :-P
>
> How should I approach this? Would you like the conversion in a single
> patch?
It will be good if you do conversion in separate patch
Regard's
Ravi Kumar V
> I think maybe adding one patch after #2 for converting the internal API and
> another one on the public API could keep the series easier to
> follow/review.
>
> --
> Gustavo Sousa
>
>>> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>>> ---
>>> drivers/gpu/drm/i915/display/intel_bw.c | 106 ++++++++++++++----------
>>> 1 file changed, 60 insertions(+), 46 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
>>> index 23edc81741de..0fb41e6cd8ae 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_bw.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
>>> @@ -124,8 +124,9 @@ static int adls_pcode_read_psf_gv_point_info(struct drm_i915_private *dev_priv,
>>>
>>> static u16 icl_qgv_points_mask(struct drm_i915_private *i915)
>>> {
>>> - unsigned int num_psf_gv_points = i915->display.bw.max[0].num_psf_gv_points;
>>> - unsigned int num_qgv_points = i915->display.bw.max[0].num_qgv_points;
>>> + struct intel_display *display = &i915->display;
>>> + unsigned int num_psf_gv_points = display->bw.max[0].num_psf_gv_points;
>>> + unsigned int num_qgv_points = display->bw.max[0].num_qgv_points;
>>> u16 qgv_points = 0, psf_points = 0;
>>>
>>> /*
>>> @@ -151,6 +152,7 @@ static bool is_sagv_enabled(struct drm_i915_private *i915, u16 points_mask)
>>> int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv,
>>> u32 points_mask)
>>> {
>>> + struct intel_display *display = &dev_priv->display;
>>> int ret;
>>>
>>> if (DISPLAY_VER(dev_priv) >= 14)
>>> @@ -170,7 +172,7 @@ int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv,
>>> return ret;
>>> }
>>>
>>> - dev_priv->display.sagv.status = is_sagv_enabled(dev_priv, points_mask) ?
>>> + display->sagv.status = is_sagv_enabled(dev_priv, points_mask) ?
>>> I915_SAGV_ENABLED : I915_SAGV_DISABLED;
>>>
>>> return 0;
>>> @@ -400,13 +402,14 @@ static const struct intel_sa_info xe2_hpd_sa_info = {
>>>
>>> static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel_sa_info *sa)
>>> {
>>> + struct intel_display *display = &dev_priv->display;
>>> struct intel_qgv_info qi = {};
>>> bool is_y_tile = true; /* assume y tile may be used */
>>> int num_channels = max_t(u8, 1, dev_priv->dram_info.num_channels);
>>> int ipqdepth, ipqdepthpch = 16;
>>> int dclk_max;
>>> int maxdebw;
>>> - int num_groups = ARRAY_SIZE(dev_priv->display.bw.max);
>>> + int num_groups = ARRAY_SIZE(display->bw.max);
>>> int i, ret;
>>>
>>> ret = icl_get_qgv_points(dev_priv, &qi, is_y_tile);
>>> @@ -422,7 +425,7 @@ static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
>>> qi.deinterleave = DIV_ROUND_UP(num_channels, is_y_tile ? 4 : 2);
>>>
>>> for (i = 0; i < num_groups; i++) {
>>> - struct intel_bw_info *bi = &dev_priv->display.bw.max[i];
>>> + struct intel_bw_info *bi = &display->bw.max[i];
>>> int clpchgroup;
>>> int j;
>>>
>>> @@ -460,15 +463,16 @@ static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
>>> * as it will fail and pointless anyway.
>>> */
>>> if (qi.num_points == 1)
>>> - dev_priv->display.sagv.status = I915_SAGV_NOT_CONTROLLED;
>>> + display->sagv.status = I915_SAGV_NOT_CONTROLLED;
>>> else
>>> - dev_priv->display.sagv.status = I915_SAGV_ENABLED;
>>> + display->sagv.status = I915_SAGV_ENABLED;
>>>
>>> return 0;
>>> }
>>>
>>> static int tgl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel_sa_info *sa)
>>> {
>>> + struct intel_display *display = &dev_priv->display;
>>> struct intel_qgv_info qi = {};
>>> const struct dram_info *dram_info = &dev_priv->dram_info;
>>> bool is_y_tile = true; /* assume y tile may be used */
>>> @@ -477,7 +481,7 @@ static int tgl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
>>> int dclk_max;
>>> int maxdebw, peakbw;
>>> int clperchgroup;
>>> - int num_groups = ARRAY_SIZE(dev_priv->display.bw.max);
>>> + int num_groups = ARRAY_SIZE(display->bw.max);
>>> int i, ret;
>>>
>>> ret = icl_get_qgv_points(dev_priv, &qi, is_y_tile);
>>> @@ -514,7 +518,7 @@ static int tgl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
>>> clperchgroup = 4 * DIV_ROUND_UP(8, num_channels) * qi.deinterleave;
>>>
>>> for (i = 0; i < num_groups; i++) {
>>> - struct intel_bw_info *bi = &dev_priv->display.bw.max[i];
>>> + struct intel_bw_info *bi = &display->bw.max[i];
>>> struct intel_bw_info *bi_next;
>>> int clpchgroup;
>>> int j;
>>> @@ -522,7 +526,7 @@ static int tgl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
>>> clpchgroup = (sa->deburst * qi.deinterleave / num_channels) << i;
>>>
>>> if (i < num_groups - 1) {
>>> - bi_next = &dev_priv->display.bw.max[i + 1];
>>> + bi_next = &display->bw.max[i + 1];
>>>
>>> if (clpchgroup < clperchgroup)
>>> bi_next->num_planes = (ipqdepth - clpchgroup) /
>>> @@ -577,17 +581,18 @@ static int tgl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
>>> * as it will fail and pointless anyway.
>>> */
>>> if (qi.num_points == 1)
>>> - dev_priv->display.sagv.status = I915_SAGV_NOT_CONTROLLED;
>>> + display->sagv.status = I915_SAGV_NOT_CONTROLLED;
>>> else
>>> - dev_priv->display.sagv.status = I915_SAGV_ENABLED;
>>> + display->sagv.status = I915_SAGV_ENABLED;
>>>
>>> return 0;
>>> }
>>>
>>> static void dg2_get_bw_info(struct drm_i915_private *i915)
>>> {
>>> + struct intel_display *display = &i915->display;
>>> unsigned int deratedbw = IS_DG2_G11(i915) ? 38000 : 50000;
>>> - int num_groups = ARRAY_SIZE(i915->display.bw.max);
>>> + int num_groups = ARRAY_SIZE(display->bw.max);
>>> int i;
>>>
>>> /*
>>> @@ -598,7 +603,7 @@ static void dg2_get_bw_info(struct drm_i915_private *i915)
>>> * whereas DG2-G11 platforms have 38 GB/s.
>>> */
>>> for (i = 0; i < num_groups; i++) {
>>> - struct intel_bw_info *bi = &i915->display.bw.max[i];
>>> + struct intel_bw_info *bi = &display->bw.max[i];
>>>
>>> bi->num_planes = 1;
>>> /* Need only one dummy QGV point per group */
>>> @@ -606,12 +611,13 @@ static void dg2_get_bw_info(struct drm_i915_private *i915)
>>> bi->deratedbw[0] = deratedbw;
>>> }
>>>
>>> - i915->display.sagv.status = I915_SAGV_NOT_CONTROLLED;
>>> + display->sagv.status = I915_SAGV_NOT_CONTROLLED;
>>> }
>>>
>>> static int xe2_hpd_get_bw_info(struct drm_i915_private *i915,
>>> const struct intel_sa_info *sa)
>>> {
>>> + struct intel_display *display = &i915->display;
>>> struct intel_qgv_info qi = {};
>>> int num_channels = i915->dram_info.num_channels;
>>> int peakbw, maxdebw;
>>> @@ -631,28 +637,28 @@ static int xe2_hpd_get_bw_info(struct drm_i915_private *i915,
>>> const struct intel_qgv_point *point = &qi.points[i];
>>> int bw = num_channels * (qi.channel_width / 8) * point->dclk;
>>>
>>> - i915->display.bw.max[0].deratedbw[i] =
>>> + display->bw.max[0].deratedbw[i] =
>>> min(maxdebw, (100 - sa->derating) * bw / 100);
>>> - i915->display.bw.max[0].peakbw[i] = bw;
>>> + display->bw.max[0].peakbw[i] = bw;
>>>
>>> drm_dbg_kms(&i915->drm, "QGV %d: deratedbw=%u peakbw: %u\n",
>>> - i, i915->display.bw.max[0].deratedbw[i],
>>> - i915->display.bw.max[0].peakbw[i]);
>>> + i, display->bw.max[0].deratedbw[i],
>>> + display->bw.max[0].peakbw[i]);
>>> }
>>>
>>> /* Bandwidth does not depend on # of planes; set all groups the same */
>>> - i915->display.bw.max[0].num_planes = 1;
>>> - i915->display.bw.max[0].num_qgv_points = qi.num_points;
>>> - for (i = 1; i < ARRAY_SIZE(i915->display.bw.max); i++)
>>> - memcpy(&i915->display.bw.max[i], &i915->display.bw.max[0],
>>> - sizeof(i915->display.bw.max[0]));
>>> + display->bw.max[0].num_planes = 1;
>>> + display->bw.max[0].num_qgv_points = qi.num_points;
>>> + for (i = 1; i < ARRAY_SIZE(display->bw.max); i++)
>>> + memcpy(&display->bw.max[i], &display->bw.max[0],
>>> + sizeof(display->bw.max[0]));
>>>
>>> /*
>>> * Xe2_HPD should always have exactly two QGV points representing
>>> * battery and plugged-in operation.
>>> */
>>> drm_WARN_ON(&i915->drm, qi.num_points != 2);
>>> - i915->display.sagv.status = I915_SAGV_ENABLED;
>>> + display->sagv.status = I915_SAGV_ENABLED;
>>>
>>> return 0;
>>> }
>>> @@ -660,6 +666,7 @@ static int xe2_hpd_get_bw_info(struct drm_i915_private *i915,
>>> static unsigned int icl_max_bw_index(struct drm_i915_private *dev_priv,
>>> int num_planes, int qgv_point)
>>> {
>>> + struct intel_display *display = &dev_priv->display;
>>> int i;
>>>
>>> /*
>>> @@ -667,9 +674,9 @@ static unsigned int icl_max_bw_index(struct drm_i915_private *dev_priv,
>>> */
>>> num_planes = max(1, num_planes);
>>>
>>> - for (i = 0; i < ARRAY_SIZE(dev_priv->display.bw.max); i++) {
>>> + for (i = 0; i < ARRAY_SIZE(display->bw.max); i++) {
>>> const struct intel_bw_info *bi =
>>> - &dev_priv->display.bw.max[i];
>>> + &display->bw.max[i];
>>>
>>> /*
>>> * Pcode will not expose all QGV points when
>>> @@ -688,6 +695,7 @@ static unsigned int icl_max_bw_index(struct drm_i915_private *dev_priv,
>>> static unsigned int tgl_max_bw_index(struct drm_i915_private *dev_priv,
>>> int num_planes, int qgv_point)
>>> {
>>> + struct intel_display *display = &dev_priv->display;
>>> int i;
>>>
>>> /*
>>> @@ -695,9 +703,9 @@ static unsigned int tgl_max_bw_index(struct drm_i915_private *dev_priv,
>>> */
>>> num_planes = max(1, num_planes);
>>>
>>> - for (i = ARRAY_SIZE(dev_priv->display.bw.max) - 1; i >= 0; i--) {
>>> + for (i = ARRAY_SIZE(display->bw.max) - 1; i >= 0; i--) {
>>> const struct intel_bw_info *bi =
>>> - &dev_priv->display.bw.max[i];
>>> + &display->bw.max[i];
>>>
>>> /*
>>> * Pcode will not expose all QGV points when
>>> @@ -716,8 +724,9 @@ static unsigned int tgl_max_bw_index(struct drm_i915_private *dev_priv,
>>> static unsigned int adl_psf_bw(struct drm_i915_private *dev_priv,
>>> int psf_gv_point)
>>> {
>>> + struct intel_display *display = &dev_priv->display;
>>> const struct intel_bw_info *bi =
>>> - &dev_priv->display.bw.max[0];
>>> + &display->bw.max[0];
>>>
>>> return bi->psf_bw[psf_gv_point];
>>> }
>>> @@ -725,6 +734,7 @@ static unsigned int adl_psf_bw(struct drm_i915_private *dev_priv,
>>> static unsigned int icl_qgv_bw(struct drm_i915_private *i915,
>>> int num_active_planes, int qgv_point)
>>> {
>>> + struct intel_display *display = &i915->display;
>>> unsigned int idx;
>>>
>>> if (DISPLAY_VER(i915) >= 12)
>>> @@ -732,10 +742,10 @@ static unsigned int icl_qgv_bw(struct drm_i915_private *i915,
>>> else
>>> idx = icl_max_bw_index(i915, num_active_planes, qgv_point);
>>>
>>> - if (idx >= ARRAY_SIZE(i915->display.bw.max))
>>> + if (idx >= ARRAY_SIZE(display->bw.max))
>>> return 0;
>>>
>>> - return i915->display.bw.max[idx].deratedbw[qgv_point];
>>> + return display->bw.max[idx].deratedbw[qgv_point];
>>> }
>>>
>>> void intel_bw_init_hw(struct drm_i915_private *dev_priv)
>>> @@ -854,10 +864,10 @@ static unsigned int intel_bw_data_rate(struct drm_i915_private *dev_priv,
>>> struct intel_bw_state *
>>> intel_atomic_get_old_bw_state(struct intel_atomic_state *state)
>>> {
>>> - struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>>> + struct intel_display *display = to_intel_display(state);
>>> struct intel_global_state *bw_state;
>>>
>>> - bw_state = intel_atomic_get_old_global_obj_state(state, &dev_priv->display.bw.obj);
>>> + bw_state = intel_atomic_get_old_global_obj_state(state, &display->bw.obj);
>>>
>>> return to_intel_bw_state(bw_state);
>>> }
>>> @@ -865,10 +875,10 @@ intel_atomic_get_old_bw_state(struct intel_atomic_state *state)
>>> struct intel_bw_state *
>>> intel_atomic_get_new_bw_state(struct intel_atomic_state *state)
>>> {
>>> - struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>>> + struct intel_display *display = to_intel_display(state);
>>> struct intel_global_state *bw_state;
>>>
>>> - bw_state = intel_atomic_get_new_global_obj_state(state, &dev_priv->display.bw.obj);
>>> + bw_state = intel_atomic_get_new_global_obj_state(state, &display->bw.obj);
>>>
>>> return to_intel_bw_state(bw_state);
>>> }
>>> @@ -876,10 +886,10 @@ intel_atomic_get_new_bw_state(struct intel_atomic_state *state)
>>> struct intel_bw_state *
>>> intel_atomic_get_bw_state(struct intel_atomic_state *state)
>>> {
>>> - struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>>> + struct intel_display *display = to_intel_display(state);
>>> struct intel_global_state *bw_state;
>>>
>>> - bw_state = intel_atomic_get_global_obj_state(state, &dev_priv->display.bw.obj);
>>> + bw_state = intel_atomic_get_global_obj_state(state, &display->bw.obj);
>>> if (IS_ERR(bw_state))
>>> return ERR_CAST(bw_state);
>>>
>>> @@ -889,7 +899,8 @@ intel_atomic_get_bw_state(struct intel_atomic_state *state)
>>> static unsigned int icl_max_bw_qgv_point_mask(struct drm_i915_private *i915,
>>> int num_active_planes)
>>> {
>>> - unsigned int num_qgv_points = i915->display.bw.max[0].num_qgv_points;
>>> + struct intel_display *display = &i915->display;
>>> + unsigned int num_qgv_points = display->bw.max[0].num_qgv_points;
>>> unsigned int max_bw_point = 0;
>>> unsigned int max_bw = 0;
>>> int i;
>>> @@ -925,7 +936,8 @@ static u16 icl_prepare_qgv_points_mask(struct drm_i915_private *i915,
>>>
>>> static unsigned int icl_max_bw_psf_gv_point_mask(struct drm_i915_private *i915)
>>> {
>>> - unsigned int num_psf_gv_points = i915->display.bw.max[0].num_psf_gv_points;
>>> + struct intel_display *display = &i915->display;
>>> + unsigned int num_psf_gv_points = display->bw.max[0].num_psf_gv_points;
>>> unsigned int max_bw_point_mask = 0;
>>> unsigned int max_bw = 0;
>>> int i;
>>> @@ -965,8 +977,9 @@ static int mtl_find_qgv_points(struct drm_i915_private *i915,
>>> unsigned int num_active_planes,
>>> struct intel_bw_state *new_bw_state)
>>> {
>>> + struct intel_display *display = &i915->display;
>>> unsigned int best_rate = UINT_MAX;
>>> - unsigned int num_qgv_points = i915->display.bw.max[0].num_qgv_points;
>>> + unsigned int num_qgv_points = display->bw.max[0].num_qgv_points;
>>> unsigned int qgv_peak_bw = 0;
>>> int i;
>>> int ret;
>>> @@ -995,17 +1008,17 @@ static int mtl_find_qgv_points(struct drm_i915_private *i915,
>>> tgl_max_bw_index(i915, num_active_planes, i);
>>> unsigned int max_data_rate;
>>>
>>> - if (bw_index >= ARRAY_SIZE(i915->display.bw.max))
>>> + if (bw_index >= ARRAY_SIZE(display->bw.max))
>>> continue;
>>>
>>> - max_data_rate = i915->display.bw.max[bw_index].deratedbw[i];
>>> + max_data_rate = display->bw.max[bw_index].deratedbw[i];
>>>
>>> if (max_data_rate < data_rate)
>>> continue;
>>>
>>> if (max_data_rate - data_rate < best_rate) {
>>> best_rate = max_data_rate - data_rate;
>>> - qgv_peak_bw = i915->display.bw.max[bw_index].peakbw[i];
>>> + qgv_peak_bw = display->bw.max[bw_index].peakbw[i];
>>> }
>>>
>>> drm_dbg_kms(&i915->drm, "QGV point %d: max bw %d required %d qgv_peak_bw: %d\n",
>>> @@ -1037,8 +1050,9 @@ static int icl_find_qgv_points(struct drm_i915_private *i915,
>>> const struct intel_bw_state *old_bw_state,
>>> struct intel_bw_state *new_bw_state)
>>> {
>>> - unsigned int num_psf_gv_points = i915->display.bw.max[0].num_psf_gv_points;
>>> - unsigned int num_qgv_points = i915->display.bw.max[0].num_qgv_points;
>>> + struct intel_display *display = &i915->display;
>>> + unsigned int num_psf_gv_points = display->bw.max[0].num_psf_gv_points;
>>> + unsigned int num_qgv_points = display->bw.max[0].num_qgv_points;
>>> u16 psf_points = 0;
>>> u16 qgv_points = 0;
>>> int i;
>>> --
>>> 2.48.1
>> --
>> Ville Syrjälä
>> Intel
More information about the Intel-xe
mailing list