[PATCH 1/2] drm/i915: Force a LUT update in intel_initial_commit()

[PATCH 1/2] drm/i915: Force a LUT update in intel_initial_commit()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

If we force a plane update to fix up our half populated plane state
we'll also force on the pipe gamma for the plane (since we always
enable pipe gamma currently). If the BIOS hasn't programmed a sensible
LUT into the hardware this will cause the image to become corrupted.
Typical symptoms are a purple/yellow/etc. flash when the driver loads.

To avoid this let's program something sensible into the LUT when
we do the plane update. In the future I plan to add proper plane
gamma enable readout so this is just a temporary measure.

Cc: Hans de Goede <hdegoede@redhat.com>
Fixes: 516a49cc1946 ("drm/i915: Fix assert_plane() warning on bootup with external display")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 132e978227fb..60c1e54285c1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15011,6 +15011,14 @@ static int intel_initial_commit(struct drm_device *dev)
 			ret = drm_atomic_add_affected_planes(state, crtc);
 			if (ret)
 				goto out;
+
+			/*
+			 * FIXME hack to force a LUT update to avoid the
+			 * plane update forcing the pipe gamma on without
+			 * having a proper LUT loaded. Remove once we
+			 * have readout for pipe gamma enable.
+			 */
+			crtc_state->color_mgmt_changed = true;
 		}
 	}
 
-- 
2.18.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: Add rotation readout for plane initial config

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

If we need to force a full plane update before userspace/fbdev
have given us a proper plane state we should try to maintain the
current plane state as much as possible (apart from the parts
of the state we're trying to fix up with the plane update).
To that end add basic readout for the plane rotation and
maintain it during the initial fb takeover.

Cc: Hans de Goede <hdegoede@redhat.com>
Fixes: 516a49cc1946 ("drm/i915: Fix assert_plane() warning on bootup with external display")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 2 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 60c1e54285c1..4d339f54559c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2831,6 +2831,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	return;
 
 valid_fb:
+	intel_state->base.rotation = plane_config->rotation;
 	intel_fill_fb_ggtt_view(&intel_state->view, fb,
 				intel_state->base.rotation);
 	intel_state->color_plane[0].stride =
@@ -7787,8 +7788,15 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
 			plane_config->tiling = I915_TILING_X;
 			fb->modifier = I915_FORMAT_MOD_X_TILED;
 		}
+
+		if (val & DISPPLANE_ROTATE_180)
+			plane_config->rotation = DRM_MODE_ROTATE_180;
 	}
 
+	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B &&
+	    val & DISPPLANE_MIRROR)
+		plane_config->rotation |= DRM_MODE_REFLECT_X;
+
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
 	fourcc = i9xx_format_to_fourcc(pixel_format);
 	fb->format = drm_format_info(fourcc);
@@ -8898,6 +8906,25 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 		goto error;
 	}
 
+	switch (val & PLANE_CTL_ROTATE_MASK) {
+	case PLANE_CTL_ROTATE_0:
+		plane_config->rotation = DRM_MODE_ROTATE_0;
+		break;
+	case PLANE_CTL_ROTATE_90:
+		plane_config->rotation = DRM_MODE_ROTATE_270;
+		break;
+	case PLANE_CTL_ROTATE_180:
+		plane_config->rotation = DRM_MODE_ROTATE_180;
+		break;
+	case PLANE_CTL_ROTATE_270:
+		plane_config->rotation = DRM_MODE_ROTATE_90;
+		break;
+	}
+
+	if (INTEL_GEN(dev_priv) >= 10 &&
+	    val & PLANE_CTL_FLIP_HORIZONTAL)
+		plane_config->rotation |= DRM_MODE_REFLECT_X;
+
 	base = I915_READ(PLANE_SURF(pipe, plane_id)) & 0xfffff000;
 	plane_config->base = base;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f575ba2a59da..a7d9ac912125 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -572,6 +572,7 @@ struct intel_initial_plane_config {
 	unsigned int tiling;
 	int size;
 	u32 base;
+	u8 rotation;
 };
 
 #define SKL_MIN_SRC_W 8
-- 
2.18.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Force a LUT update in intel_initial_commit()

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Force a LUT update in intel_initial_commit()
URL   : https://patchwork.freedesktop.org/series/52754/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5172 -> Patchwork_10861 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/52754/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10861:

  === IGT changes ===

    ==== Possible regressions ====

    {igt@runner@aborted}:
      {fi-icl-y}:         NOTRUN -> FAIL

    
== Known issues ==

  Here are the changes found in Patchwork_10861 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ctx_create@basic-files:
      fi-icl-u2:          PASS -> DMESG-WARN (fdo#107724)

    igt@gem_exec_suspend@basic-s4-devices:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    
    ==== Possible fixes ====

    igt@gem_ctx_switch@basic-default:
      fi-icl-u2:          DMESG-WARN (fdo#107724) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       DMESG-WARN (fdo#102614) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-kbl-7560u:       FAIL (fdo#103375) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724


== Participating hosts (51 -> 46) ==

  Additional (1): fi-icl-y 
  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u3 


== Build changes ==

    * Linux: CI_DRM_5172 -> Patchwork_10861

  CI_DRM_5172: 469c356dc6c310bed869703bfc75a56cd63075b5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4721: 68264891ceee34195839d82d4d87cbae08ef2431 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10861: e9005dd0153e45c85de821d4fc30ea89ec4e1e04 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e9005dd0153e drm/i915: Add rotation readout for plane initial config
22d340f39fee drm/i915: Force a LUT update in intel_initial_commit()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10861/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Force a LUT update in intel_initial_commit()

[Hans de Goede]

Hi Ville,

On 20-11-18 14:54, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> If we force a plane update to fix up our half populated plane state
> we'll also force on the pipe gamma for the plane (since we always
> enable pipe gamma currently). If the BIOS hasn't programmed a sensible
> LUT into the hardware this will cause the image to become corrupted.
> Typical symptoms are a purple/yellow/etc. flash when the driver loads.
> 
> To avoid this let's program something sensible into the LUT when
> we do the plane update. In the future I plan to add proper plane
> gamma enable readout so this is just a temporary measure.
> 
> Cc: Hans de Goede <hdegoede@redhat.com>
> Fixes: 516a49cc1946 ("drm/i915: Fix assert_plane() warning on bootup with external display")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I can confirm that this series fixes the purple flash I was seeing on
DSI panels.

And the reporter in bug https://bugs.freedesktop.org/show_bug.cgi?id=108225
has also just reported back that the blanking he was seeing which was
also caused by the "drm/i915: Fix assert_plane() warning on bootup with external display"
commit is also fixed, so it looks like this series is good to go:

Tested-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans




> ---
>   drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 132e978227fb..60c1e54285c1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15011,6 +15011,14 @@ static int intel_initial_commit(struct drm_device *dev)
>   			ret = drm_atomic_add_affected_planes(state, crtc);
>   			if (ret)
>   				goto out;
> +
> +			/*
> +			 * FIXME hack to force a LUT update to avoid the
> +			 * plane update forcing the pipe gamma on without
> +			 * having a proper LUT loaded. Remove once we
> +			 * have readout for pipe gamma enable.
> +			 */
> +			crtc_state->color_mgmt_changed = true;
>   		}
>   	}
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Force a LUT update in intel_initial_commit()

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Force a LUT update in intel_initial_commit()
URL   : https://patchwork.freedesktop.org/series/52754/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5172_full -> Patchwork_10861_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10861_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10861_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10861_full:

  === IGT changes ===

    ==== Possible regressions ====

    {igt@runner@aborted}:
      {shard-iclb}:       NOTRUN -> FAIL

    
    ==== Warnings ====

    igt@perf_pmu@rc6:
      shard-kbl:          SKIP -> PASS

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          SKIP -> PASS

    igt@tools_test@tools_test:
      shard-snb:          PASS -> SKIP
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_10861_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_workarounds@suspend-resume-context:
      shard-skl:          NOTRUN -> INCOMPLETE (fdo#104108, fdo#107773)

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
      shard-glk:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
      {shard-iclb}:       PASS -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
      {shard-iclb}:       NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_cursor_crc@cursor-64x64-dpms:
      shard-glk:          PASS -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-size-change:
      shard-apl:          PASS -> FAIL (fdo#103232) +1

    igt@kms_draw_crc@draw-method-xrgb2101010-render-xtiled:
      shard-skl:          PASS -> FAIL (fdo#103184) +1

    igt@kms_flip@dpms-vs-vblank-race-interruptible:
      shard-glk:          PASS -> FAIL (fdo#103060)

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-skl:          PASS -> FAIL (fdo#105363)

    igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
      shard-glk:          PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
      shard-apl:          PASS -> FAIL (fdo#103167) +1

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
      shard-skl:          NOTRUN -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-render:
      shard-skl:          PASS -> FAIL (fdo#103167) +5

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
      shard-skl:          PASS -> FAIL (fdo#105682)

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-pwrite:
      {shard-iclb}:       PASS -> FAIL (fdo#103167) +2

    igt@kms_plane@plane-position-covered-pipe-b-planes:
      shard-apl:          PASS -> FAIL (fdo#103166)

    igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
      shard-apl:          NOTRUN -> FAIL (fdo#108145)

    igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
      shard-skl:          PASS -> FAIL (fdo#107815, fdo#108145)

    igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
      shard-skl:          NOTRUN -> FAIL (fdo#107815, fdo#108145)

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-glk:          PASS -> FAIL (fdo#103166)
      {shard-iclb}:       PASS -> FAIL (fdo#103166) +1

    igt@kms_plane_scaling@pipe-b-scaler-with-pixel-format:
      {shard-iclb}:       NOTRUN -> DMESG-WARN (fdo#107724) +1

    igt@pm_backlight@basic-brightness:
      {shard-iclb}:       PASS -> DMESG-WARN (fdo#107724) +2

    igt@pm_rpm@modeset-pc8-residency-stress:
      shard-skl:          SKIP -> INCOMPLETE (fdo#107807)

    igt@pm_rpm@pm-tiling:
      shard-skl:          PASS -> INCOMPLETE (fdo#107807)

    
    ==== Possible fixes ====

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-apl:          INCOMPLETE (fdo#103927) -> PASS

    igt@gem_userptr_blits@readonly-unsync:
      shard-glk:          INCOMPLETE (k.org#198133, fdo#103359) -> PASS

    igt@kms_atomic_transition@plane-all-transition-nonblocking-fencing:
      shard-apl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS +5

    igt@kms_chv_cursor_fail@pipe-c-64x64-bottom-edge:
      shard-skl:          FAIL (fdo#104671) -> PASS

    igt@kms_color@pipe-c-ctm-green-to-red:
      shard-skl:          FAIL (fdo#107201) -> PASS

    igt@kms_color@pipe-c-degamma:
      shard-skl:          FAIL (fdo#104782) -> PASS

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-skl:          INCOMPLETE (fdo#104108, fdo#107773) -> PASS +1

    igt@kms_cursor_crc@cursor-256x85-random:
      shard-glk:          FAIL (fdo#103232) -> PASS +2

    igt@kms_cursor_crc@cursor-64x21-onscreen:
      shard-apl:          FAIL (fdo#103232) -> PASS

    igt@kms_flip@flip-vs-expired-vblank:
      shard-skl:          FAIL (fdo#105363) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
      shard-glk:          FAIL (fdo#103167) -> PASS +2

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
      {shard-iclb}:       FAIL (fdo#103167) -> PASS +3

    igt@kms_plane@plane-panning-top-left-pipe-b-planes:
      shard-skl:          FAIL (fdo#103166) -> PASS

    igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
      shard-glk:          FAIL (fdo#108145) -> PASS

    igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
      shard-skl:          FAIL (fdo#107815) -> PASS

    igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
      {shard-iclb}:       FAIL (fdo#103166) -> PASS +1

    igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
      shard-apl:          FAIL (fdo#103166) -> PASS +1

    igt@kms_psr@no_drrs:
      {shard-iclb}:       FAIL (fdo#108341) -> PASS

    igt@kms_setmode@basic:
      shard-kbl:          FAIL (fdo#99912) -> PASS

    igt@pm_rpm@dpms-non-lpsp:
      shard-skl:          INCOMPLETE (fdo#107807) -> SKIP

    
    ==== Warnings ====

    igt@kms_frontbuffer_tracking@fbc-1p-rte:
      shard-apl:          DMESG-FAIL (fdo#105602, fdo#103558, fdo#103167) -> FAIL (fdo#105682, fdo#103167)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
  fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#107201 https://bugs.freedesktop.org/show_bug.cgi?id=107201
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108341 https://bugs.freedesktop.org/show_bug.cgi?id=108341
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (7 -> 7) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5172 -> Patchwork_10861

  CI_DRM_5172: 469c356dc6c310bed869703bfc75a56cd63075b5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4721: 68264891ceee34195839d82d4d87cbae08ef2431 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10861: e9005dd0153e45c85de821d4fc30ea89ec4e1e04 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10861/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Add rotation readout for plane initial config

[Rodrigo Vivi]

On Tue, Nov 20, 2018 at 03:54:50PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> If we need to force a full plane update before userspace/fbdev
> have given us a proper plane state we should try to maintain the
> current plane state as much as possible (apart from the parts
> of the state we're trying to fix up with the plane update).
> To that end add basic readout for the plane rotation and
> maintain it during the initial fb takeover.
> 
> Cc: Hans de Goede <hdegoede@redhat.com>
> Fixes: 516a49cc1946 ("drm/i915: Fix assert_plane() warning on bootup with external display")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 60c1e54285c1..4d339f54559c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2831,6 +2831,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>  	return;
>  
>  valid_fb:
> +	intel_state->base.rotation = plane_config->rotation;
>  	intel_fill_fb_ggtt_view(&intel_state->view, fb,
>  				intel_state->base.rotation);
>  	intel_state->color_plane[0].stride =
> @@ -7787,8 +7788,15 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
>  			plane_config->tiling = I915_TILING_X;
>  			fb->modifier = I915_FORMAT_MOD_X_TILED;
>  		}
> +
> +		if (val & DISPPLANE_ROTATE_180)
> +			plane_config->rotation = DRM_MODE_ROTATE_180;
>  	}
>  
> +	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B &&
> +	    val & DISPPLANE_MIRROR)
> +		plane_config->rotation |= DRM_MODE_REFLECT_X;
> +
>  	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
>  	fourcc = i9xx_format_to_fourcc(pixel_format);
>  	fb->format = drm_format_info(fourcc);
> @@ -8898,6 +8906,25 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
>  		goto error;
>  	}
>  
> +	switch (val & PLANE_CTL_ROTATE_MASK) {
> +	case PLANE_CTL_ROTATE_0:
> +		plane_config->rotation = DRM_MODE_ROTATE_0;
> +		break;
> +	case PLANE_CTL_ROTATE_90:
> +		plane_config->rotation = DRM_MODE_ROTATE_270;

we should add that comment that DRM_MODE_ROTATE_ is counter clockwise
to avoid later confusion.

Also I wonder if we shouldn't move all the cases to a unique
map function.

Anyways,


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



> +		break;
> +	case PLANE_CTL_ROTATE_180:
> +		plane_config->rotation = DRM_MODE_ROTATE_180;
> +		break;
> +	case PLANE_CTL_ROTATE_270:
> +		plane_config->rotation = DRM_MODE_ROTATE_90;
> +		break;
> +	}
> +
> +	if (INTEL_GEN(dev_priv) >= 10 &&
> +	    val & PLANE_CTL_FLIP_HORIZONTAL)
> +		plane_config->rotation |= DRM_MODE_REFLECT_X;
> +
>  	base = I915_READ(PLANE_SURF(pipe, plane_id)) & 0xfffff000;
>  	plane_config->base = base;
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f575ba2a59da..a7d9ac912125 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -572,6 +572,7 @@ struct intel_initial_plane_config {
>  	unsigned int tiling;
>  	int size;
>  	u32 base;
> +	u8 rotation;
>  };
>  
>  #define SKL_MIN_SRC_W 8
> -- 
> 2.18.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Force a LUT update in intel_initial_commit()

[Rodrigo Vivi]

On Tue, Nov 20, 2018 at 03:54:49PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> If we force a plane update to fix up our half populated plane state
> we'll also force on the pipe gamma for the plane (since we always
> enable pipe gamma currently). If the BIOS hasn't programmed a sensible
> LUT into the hardware this will cause the image to become corrupted.
> Typical symptoms are a purple/yellow/etc. flash when the driver loads.
> 
> To avoid this let's program something sensible into the LUT when
> we do the plane update. In the future I plan to add proper plane
> gamma enable readout so this is just a temporary measure.

Since this is temporary and marked with FIXME and Hans confirmed
it solves the best behaviour let's move with this.

But I'm wondering if there is no way to minimize this instead of
forcing this to every commit for each crtc...

Anyway,

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> Cc: Hans de Goede <hdegoede@redhat.com>
> Fixes: 516a49cc1946 ("drm/i915: Fix assert_plane() warning on bootup with external display")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 132e978227fb..60c1e54285c1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15011,6 +15011,14 @@ static int intel_initial_commit(struct drm_device *dev)
>  			ret = drm_atomic_add_affected_planes(state, crtc);
>  			if (ret)
>  				goto out;
> +
> +			/*
> +			 * FIXME hack to force a LUT update to avoid the
> +			 * plane update forcing the pipe gamma on without
> +			 * having a proper LUT loaded. Remove once we
> +			 * have readout for pipe gamma enable.
> +			 */
> +			crtc_state->color_mgmt_changed = true;
>  		}
>  	}
>  
> -- 
> 2.18.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Add rotation readout for plane initial config

[Maarten Lankhorst]

Op 20-11-18 om 23:20 schreef Rodrigo Vivi:
> On Tue, Nov 20, 2018 at 03:54:50PM +0200, Ville Syrjala wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> If we need to force a full plane update before userspace/fbdev
>> have given us a proper plane state we should try to maintain the
>> current plane state as much as possible (apart from the parts
>> of the state we're trying to fix up with the plane update).
>> To that end add basic readout for the plane rotation and
>> maintain it during the initial fb takeover.
>>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Fixes: 516a49cc1946 ("drm/i915: Fix assert_plane() warning on bootup with external display")
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 60c1e54285c1..4d339f54559c 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -2831,6 +2831,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>>  	return;
>>  
>>  valid_fb:
>> +	intel_state->base.rotation = plane_config->rotation;
>>  	intel_fill_fb_ggtt_view(&intel_state->view, fb,
>>  				intel_state->base.rotation);
>>  	intel_state->color_plane[0].stride =
>> @@ -7787,8 +7788,15 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
>>  			plane_config->tiling = I915_TILING_X;
>>  			fb->modifier = I915_FORMAT_MOD_X_TILED;
>>  		}
>> +
>> +		if (val & DISPPLANE_ROTATE_180)
>> +			plane_config->rotation = DRM_MODE_ROTATE_180;
>>  	}
>>  
>> +	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B &&
>> +	    val & DISPPLANE_MIRROR)
>> +		plane_config->rotation |= DRM_MODE_REFLECT_X;
>> +
>>  	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
>>  	fourcc = i9xx_format_to_fourcc(pixel_format);
>>  	fb->format = drm_format_info(fourcc);
>> @@ -8898,6 +8906,25 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
>>  		goto error;
>>  	}
>>  
>> +	switch (val & PLANE_CTL_ROTATE_MASK) {
>> +	case PLANE_CTL_ROTATE_0:
>> +		plane_config->rotation = DRM_MODE_ROTATE_0;
>> +		break;
>> +	case PLANE_CTL_ROTATE_90:
>> +		plane_config->rotation = DRM_MODE_ROTATE_270;
> we should add that comment that DRM_MODE_ROTATE_ is counter clockwise
> to avoid later confusion.
>
> Also I wonder if we shouldn't move all the cases to a unique
> map function.
>
> Anyways,
>
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
>
>
>> +		break;
>> +	case PLANE_CTL_ROTATE_180:
>> +		plane_config->rotation = DRM_MODE_ROTATE_180;
>> +		break;
>> +	case PLANE_CTL_ROTATE_270:
>> +		plane_config->rotation = DRM_MODE_ROTATE_90;
>> +		break;
>> +	}
>> +
>> +	if (INTEL_GEN(dev_priv) >= 10 &&
>> +	    val & PLANE_CTL_FLIP_HORIZONTAL)
>> +		plane_config->rotation |= DRM_MODE_REFLECT_X;
>> +
>>  	base = I915_READ(PLANE_SURF(pipe, plane_id)) & 0xfffff000;
>>  	plane_config->base = base;
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index f575ba2a59da..a7d9ac912125 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -572,6 +572,7 @@ struct intel_initial_plane_config {
>>  	unsigned int tiling;
>>  	int size;
>>  	u32 base;
>> +	u8 rotation;
>>  };
>>  
>>  #define SKL_MIN_SRC_W 8
>> -- 
>> 2.18.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Not sure if pushed yet.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Force a LUT update in intel_initial_commit()

[Ville Syrjälä]

On Tue, Nov 20, 2018 at 02:25:40PM -0800, Rodrigo Vivi wrote:
> On Tue, Nov 20, 2018 at 03:54:49PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > If we force a plane update to fix up our half populated plane state
> > we'll also force on the pipe gamma for the plane (since we always
> > enable pipe gamma currently). If the BIOS hasn't programmed a sensible
> > LUT into the hardware this will cause the image to become corrupted.
> > Typical symptoms are a purple/yellow/etc. flash when the driver loads.
> > 
> > To avoid this let's program something sensible into the LUT when
> > we do the plane update. In the future I plan to add proper plane
> > gamma enable readout so this is just a temporary measure.
> 
> Since this is temporary and marked with FIXME and Hans confirmed
> it solves the best behaviour let's move with this.
> 
> But I'm wondering if there is no way to minimize this instead of
> forcing this to every commit for each crtc...

It's just for the first commit we do when loading the driver.

> 
> Anyway,
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> > 
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Fixes: 516a49cc1946 ("drm/i915: Fix assert_plane() warning on bootup with external display")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 132e978227fb..60c1e54285c1 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -15011,6 +15011,14 @@ static int intel_initial_commit(struct drm_device *dev)
> >  			ret = drm_atomic_add_affected_planes(state, crtc);
> >  			if (ret)
> >  				goto out;
> > +
> > +			/*
> > +			 * FIXME hack to force a LUT update to avoid the
> > +			 * plane update forcing the pipe gamma on without
> > +			 * having a proper LUT loaded. Remove once we
> > +			 * have readout for pipe gamma enable.
> > +			 */
> > +			crtc_state->color_mgmt_changed = true;
> >  		}
> >  	}
> >  
> > -- 
> > 2.18.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Add rotation readout for plane initial config

[Ville Syrjälä]

On Tue, Nov 20, 2018 at 02:20:14PM -0800, Rodrigo Vivi wrote:
> On Tue, Nov 20, 2018 at 03:54:50PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > If we need to force a full plane update before userspace/fbdev
> > have given us a proper plane state we should try to maintain the
> > current plane state as much as possible (apart from the parts
> > of the state we're trying to fix up with the plane update).
> > To that end add basic readout for the plane rotation and
> > maintain it during the initial fb takeover.
> > 
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Fixes: 516a49cc1946 ("drm/i915: Fix assert_plane() warning on bootup with external display")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h     |  1 +
> >  2 files changed, 28 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 60c1e54285c1..4d339f54559c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2831,6 +2831,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> >  	return;
> >  
> >  valid_fb:
> > +	intel_state->base.rotation = plane_config->rotation;
> >  	intel_fill_fb_ggtt_view(&intel_state->view, fb,
> >  				intel_state->base.rotation);
> >  	intel_state->color_plane[0].stride =
> > @@ -7787,8 +7788,15 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
> >  			plane_config->tiling = I915_TILING_X;
> >  			fb->modifier = I915_FORMAT_MOD_X_TILED;
> >  		}
> > +
> > +		if (val & DISPPLANE_ROTATE_180)
> > +			plane_config->rotation = DRM_MODE_ROTATE_180;
> >  	}
> >  
> > +	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B &&
> > +	    val & DISPPLANE_MIRROR)
> > +		plane_config->rotation |= DRM_MODE_REFLECT_X;
> > +
> >  	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
> >  	fourcc = i9xx_format_to_fourcc(pixel_format);
> >  	fb->format = drm_format_info(fourcc);
> > @@ -8898,6 +8906,25 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> >  		goto error;
> >  	}
> >  
> > +	switch (val & PLANE_CTL_ROTATE_MASK) {
> > +	case PLANE_CTL_ROTATE_0:
> > +		plane_config->rotation = DRM_MODE_ROTATE_0;
> > +		break;
> > +	case PLANE_CTL_ROTATE_90:
> > +		plane_config->rotation = DRM_MODE_ROTATE_270;
> 
> we should add that comment that DRM_MODE_ROTATE_ is counter clockwise
> to avoid later confusion.

I copied over the comment from the other place, and pushed the series
to dinq. Thanks for the reviews, testing, and tracking down the bug
initially.

> 
> Also I wonder if we shouldn't move all the cases to a unique
> map function.
> 
> Anyways,
> 
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> 
> 
> > +		break;
> > +	case PLANE_CTL_ROTATE_180:
> > +		plane_config->rotation = DRM_MODE_ROTATE_180;
> > +		break;
> > +	case PLANE_CTL_ROTATE_270:
> > +		plane_config->rotation = DRM_MODE_ROTATE_90;
> > +		break;
> > +	}
> > +
> > +	if (INTEL_GEN(dev_priv) >= 10 &&
> > +	    val & PLANE_CTL_FLIP_HORIZONTAL)
> > +		plane_config->rotation |= DRM_MODE_REFLECT_X;
> > +
> >  	base = I915_READ(PLANE_SURF(pipe, plane_id)) & 0xfffff000;
> >  	plane_config->base = base;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index f575ba2a59da..a7d9ac912125 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -572,6 +572,7 @@ struct intel_initial_plane_config {
> >  	unsigned int tiling;
> >  	int size;
> >  	u32 base;
> > +	u8 rotation;
> >  };
> >  
> >  #define SKL_MIN_SRC_W 8
> > -- 
> > 2.18.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx