On Fri, 2026-05-22 at 23:03 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <[email protected]>
> 
> The TGL+ bw code has an off by one error on the num_planes
> calculation, and tgl_max_bw_index() incorrectly bumps
> the num_planes to 1 from 0.
> 
> That approach made sense on ICL where num_planes is more or
> a less minimum number of planes to consider for the group,
> but on TGL+ num_planes really is a maximum number of planes,
> so these adjustments no longer make any sense there.
> 
> Signed-off-by: Ville Syrjälä <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c
> b/drivers/gpu/drm/i915/display/intel_bw.c
> index d7b2bc80f8e3..d10eebec196e 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -636,8 +636,7 @@ static int tgl_get_bw_info(struct intel_display
> *display,
>                       bi_next = &display->bw.max[i + 1];
>  
>                       if (clpchgroup < clperchgroup)
> -                             bi_next->num_planes = (ipqdepth -
> clpchgroup) /
> -                                                    clpchgroup +
> 1;
> +                             bi_next->num_planes = (ipqdepth -
> clpchgroup) / clpchgroup;
>                       else
>                               bi_next->num_planes = 0;
>               }
> @@ -802,11 +801,6 @@ static unsigned int tgl_max_bw_index(struct
> intel_display *display,
>  {
>       int i;
>  
> -     /*
> -      * Let's return max bw for 0 planes
> -      */
> -     num_planes = max(1, num_planes);
> -

Not related to this patch - in continuation to disucssion we had
earlier. 

Not Sure what is the real purpose of this loop. Isn't this logic expect
the QGV points in sorted order. But in reality those not in any sorted
order (not sure if there are any such requirements) - have seen few
such unsorted qgv points in MTL, LNL etc. But most of the recent seen
ones are sorted though.

So if those are not guaranteed to sorted, what is the point
in searching in any particular order? Aren't we supposed to select the
best performance and power efficient QGV point? In this loop we will
end up in selecting the first matching point which might not the
"best". But worst is, as we are seeing num_planes as 0 in recent
platforms for almost all the bw_info points, most of the time we will
end up selecting the index 0 - which could be either the best one or
the worst one.

Thats the reason I was thinking of sorting the QGV points for pmdemand
cases. May be do we need to consider sorting the QGV points in all the
cases and include "original" qgv_index as a member of bw info to store
the "pcode" index of the qgv points for non pmdemand cases?

BR
Vinod  

>       for (i = ARRAY_SIZE(display->bw.max) - 1; i >= 0; i--) {
>               const struct intel_bw_info *bi =
>                       &display->bw.max[i];

Reply via email to