[Intel-gfx] [PATCH 05/11] drm/i915/display/dp: Compute VRR state in atomic_check

[Navare, Manasi]

On Thu, Dec 03, 2020 at 06:39:43PM +0200, Jani Nikula wrote:
> On Wed, 02 Dec 2020, "Navare, Manasi" <manasi.d.navare at intel.com> wrote:
> > On Tue, Dec 01, 2020 at 02:52:59PM -0800, Navare, Manasi wrote:
> >> On Tue, Nov 10, 2020 at 12:47:46PM +0200, Jani Nikula wrote:
> >> > On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare at intel.com> wrote:
> >> > > @@ -15202,7 +15206,8 @@ static int intel_atomic_check(struct drm_device *dev,
> >> > >  
> >> > >  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> >> > >  					    new_crtc_state, i) {
> >> > > -		if (new_crtc_state->inherited != old_crtc_state->inherited)
> >> > > +		if (new_crtc_state->inherited != old_crtc_state->inherited ||
> >> > > +		    new_crtc_state->uapi.vrr_enabled != old_crtc_state->uapi.vrr_enabled)
> >> > 
> >> > Somehow this feels like a really specific check to add considering the
> >> > abstraction level of the function in general.
> >
> > Actually while discussing with @Ville on IRC, he had proposed just adding it here
> > since we already have this loop existing that loops through the old and new crtc states
> > and we need to set the mode_changed = true right up at the top.
> > But if you think its more intuitive to create a separate function for this I could do that
> >
> > Ville, Jani N any thoughts?
> 
> It's just a gut feeling. Kind of uneasy feeling that in the future
> people look at that, and see you have this check there, and then add
> more, and more.
> 
> Anyway, whatever Ville says works for me as well. ;)

Yea I actually also agree reg this so let me just move this to a separate
function where itw ill loop through crtc states and force mode changed
for this condition.

If Ville thinks otherwise we can remove it since I havent got any review comments
from Ville yet.

Manasi

> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center