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

Jani Nikula jani.nikula at linux.intel.com
Thu Dec 3 16:39:43 UTC 2020


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. ;)

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center


More information about the Intel-gfx mailing list