On Thu, Dec 03, 2020 at 06:39:43PM +0200, Jani Nikula wrote: > On Wed, 02 Dec 2020, "Navare, Manasi" <[email protected]> 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 <[email protected]> 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 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
