On Thu, Dec 03, 2020 at 06:39:43PM +0200, Jani Nikula wrote:
> On Wed, 02 Dec 2020, "Navare, Manasi" <manasi.d.navare@xxxxxxxxx> 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@xxxxxxxxx> 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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
