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=e42aee...

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@gmail.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Lionel Landwerlin lionel.g.landwerlin@intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch

---

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

Ok, so legacy gamma table updates are completely broken for Intel on Linux-4.7-rc7, the final release candidate.

The good news is that applying Lionel's patch

"drm/i915: add missing condition for committing planes on crtc"

from

https://patchwork.freedesktop.org/patch/89111/

fixes it nicely. The patch currently applies cleanly to drm-fixes and drm-next and is

Reviewed-and-tested-by: Mario Kleiner mario.kleiner.de@gmail.com

When we are at it, could somebody please look at that updated series of my Displayport color depth fixes ("EDID/DP fixes for proper bpc detection of displays.") i sent out a week ago?

Especially pulling patch 2/5 "[PATCH 2/5] drm/i915/dp: Revert "drm/i915/dp: fall back to 18 bpp when sink capability is unknown" would be important, as that bug introduced a regression for Intel + DP + legacy DP converters into stable kernels which is very serious for users of scientific/medical display equipment, especially as the failures can easily go unnoticed during normal equipment tests, but would introduce the equivalent of "silent data corruption" into their measured scientific data, which is not a great experience given that collecting such data can easily take half a year of work time and ten-thousands of euros of wasted research funding.

Patches 3 and 4 contain changes Daniel asked me to do, patch 5 would be good to safe-guard against similar issues in the future.

thanks, -mario

On 07/12/2016 12:50 PM, Lionel Landwerlin wrote:

Hi Mario,

There was a couple of patch to fix this issue :

https://patchwork.freedesktop.org/series/5467/ https://patchwork.freedesktop.org/series/5466/

I tested this late last week on drm-intel-nightly, it seems a series of revert fixed most of the issues.

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@gmail.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Lionel Landwerlin lionel.g.landwerlin@intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch

---

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