Re: [PATCH] drm/i915: Fix legacy gamma lut updates in Linux 4.7-rc6

[Lionel Landwerlin]

On 12/07/16 13:11, Mario Kleiner wrote:
> On 07/12/2016 12:50 PM, Lionel Landwerlin wrote:
> > Hi Mario,
>
> Hi Lionel,
>
> > There was a couple of patch to fix this issue :
> >
> > https://patchwork.freedesktop.org/series/5467/
> > https://patchwork.freedesktop.org/series/5466/
>
> Looking at them they should fix the issue, but they seem to be stuck 
> in review?
>
> > I tested this late last week on drm-intel-nightly, it seems a series of
> > revert fixed most of the issues.
>
> You mean something else has fixed legacy gamma updates, as i can't 
> find above patches applied on drm-intel-nightly?

This revert on drm-intel-nightly seems to have fixed the problem :

https://cgit.freedesktop.org/drm-intel/commit/drivers/gpu/drm/i915?id=e42aeef1237b7c969a77b7f726c50f6cb832185f

> Are those fixes supposed to be already part of 4.7-rc7, the final rc 
> afaik?

I haven't seen it on 4.7-rc7.

> thanks,
> -mario
>
>
> > Cheers,
> >
> > -
> > Lionel
> >
> > On 12/07/16 11:33, Mario Kleiner wrote:
> > > Updating legacy gamma tables, e.g., via RandR doesn't work at all
> > > as of Linux 4.7-rc6.
> > >
> > > Reason seems to be that the required call to
> > > drm_atomic_helper_commit_planes_on_crtc is skipped in
> > > intel_atomic_commit after userspace set new gamma tables,
> > > because neither crtc->state->planes_changed nor
> > > update_pipe (= pipe_config->update_pipe) are true.
> > >
> > > Removing the check for planes_changed || update_pipe fixes
> > > gamma table updates.
> > >
> > > The code for Linux 4.8 drm-next has changed a lot in that area
> > > wrt. 4.7, but the new code for 4.8 also removed those checks
> > > and calls drm_atomic_helper_commit_planes_on_crtc unconditionally,
> > > and legacy gamma lut updates work on drm-next, so this seems to be
> > > the right solution.
> > >
> > > Tested also shutdown/reboot, suspend/resume, (un-)plugging displays,
> > > mode switches for resolution/refresh rate, display rotation, and
> > > page-flipping/pageflip timing on Intel HD Ironlake to confirm the
> > > fix apparently doesn't break anything under X11.
> > >
> > > Signed-off-by: Mario Kleiner <mario.kleiner.de@xxxxxxxxx>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> > > Cc: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > > ---
> > >   drivers/gpu/drm/i915/intel_display.c | 4 +---
> > >   1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 04452cf..eb8fb36 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -13685,7 +13685,6 @@ static int intel_atomic_commit(struct
> > > drm_device *dev,
> > >           bool modeset = needs_modeset(crtc->state);
> > >           struct intel_crtc_state *pipe_config =
> > >               to_intel_crtc_state(crtc->state);
> > > -        bool update_pipe = !modeset && pipe_config->update_pipe;
> > >           if (modeset && crtc->state->active) {
> > >               update_scanline_offset(to_intel_crtc(crtc));
> > > @@ -13699,8 +13698,7 @@ static int intel_atomic_commit(struct
> > > drm_device *dev,
> > >               drm_atomic_get_existing_plane_state(state, 
> > > crtc->primary))
> > >               intel_fbc_enable(intel_crtc);
> > > -        if (crtc->state->active &&
> > > -            (crtc->state->planes_changed || update_pipe))
> > > +        if (crtc->state->active)
> > > drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
> > >           if (pipe_config->base.active && 
> > > needs_vblank_wait(pipe_config))