[Intel-gfx] [PATCH v19 4/8] drm/i915: Refactor intel_can_enable_sagv

[Ville Syrjälä]

On Wed, Mar 18, 2020 at 11:52:13AM +0000, Lisovskiy, Stanislav wrote:
> >> @@ -5829,6 +6068,10 @@ skl_compute_wm(struct intel_atomic_state *state)
> >>                        return ret;
> >>        }
> >>
> >> +     ret = intel_compute_sagv_mask(state);
> >> +     if (ret)
> >> +             return ret;
> 
> > This seems too early. We haven't even computed the ddb yet.
> 
> 
> I was thinking about our discussion last week and actually I think there are simply two ways how
> 
> to do it.
> 
> 
> 1) What I do here is: calculate minimum amount required to fit SAGV wm levels into ddb and
> 
> based on that do the ddb allocation accordingly. I.e it is not to early because actually we have
> 
> already wm levels for sagv and non-sagv calculated - we already can check if it can fit into L0
> 
> and then act accordingly.
> 
> However one thing to consider here: as you said besides the minimal requirements for each plane
> 
> (with or without sagv) there is an extra space being allocated in proportion to plane data rate,
> 
> however here we are actually hitting the prioritization issue - i.e we need to decide whether
> 
> it is more important to have SAGV or to have more extra space allocated to different planes
> 
> proportionally to their needs.
> 
> So in this first approach we always first determine if we fit into minimum SAGV reqs, turn it
> 
> on if we do and then rest of extra space is allocated among the planes in proportion to data rate.
> 
> So that way we would be more often power efficient but but planes get less extra ddb space.
> 
> 
> 2) In your approach we should calculate ddb first, allocate extra space proportionally to plane
> 
> data rate needs and only then check if all planes got enough space for L0 SAGV wm after that.
> 
> Then we actually don't even need skl_plane_wm_level accessor, because we first would be allocating
> 
> using normal wm levels + extra ddb and only then check if all planes fit into SAGV requirement -
> 
> because that extra space is not actually distributed evenly but in proportion to data rate of each
> 
> plane, which means that some planes might lack space for SAGV theoretically, because some might be
> 
> getting more or less depending on the data_rate/total_data_rate ratio.
> 
> 
> My position is such that I'm really not like "my approach should always win" here, but more searching for
> 
> solution which is more correct from product point of view.
> 
> 
> Also could be that it doesn't really matter which approach we do take now,, but matter more like
> 
> that how fast we deliver.  Because the actual outcome difference between two
> 
> might be minor, while time overhead for changing the approach could be major.

Pls fix your MUA. Really hard to read this.

-- 
Ville Syrjälä
Intel