Re: [Intel-gfx] [PATCH 11/15] drm/i915: Nuke intel_bw_calc_min_cdclk()

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Feb 01, 2022 at 10:52:39AM +0200, Lisovskiy, Stanislav wrote:
> On Tue, Jan 18, 2022 at 11:23:50AM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > intel_bw_calc_min_cdclk() is entirely pointless. All it manages to do is
> > somehow conflate the per-pipe min cdclk with dbuf min cdclk. There is no
> > (at least documented) dbuf min cdclk limit on pre-skl so let's just get
> > rid of all this confusion.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I think we constantly have a bit contradictional attitude towards such situation.
> >From one perspective you can say, that those kind of "leagcy" callbacks are
> pointless, from the other hand one might say. that we need to have a unified
> approach for all platforms and I think we got, some legacy handlers for old
> platforms for similar purpose as well.
> I'm fine with both approaches, however for example when I was submitting
> that patch, I was asked by reviewer to add this kind of legacy callback, so that we have
> a "uniform" approach.
> We just then need to have some standard agreement on those, which doesn't
> depend on today's cosmic radiation levels :)

Yes in general I prefer a unified approach across all platforms.
But in this case there is nothing to do for the old platforms as they
don't have any kind of dbuf cdclk limit, or if there is one we don't
know what it is since it's not documented.

So the only thing the code was really doing was conflating the
per-pipe cdclk limit (which is handled elsewhere for all platforms
in a  unified fashion) with something that doesn't even exist.

Also I don't think it was even correct anyway since it was
using the per-pipe cdclk_state->min_cdclk[] already during
intel_cdclk_atomic_check(), but cdclk_state->min_cdclk[]
isn't even computed until intel_modeset_calc_cdclk() which 
is called later. So I guess it was basically just computing 
the max of the min_cdclk[] for all the pipes for the _old_
state, not the new state.

-- 
Ville Syrjälä
Intel