* [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state
@ 2015-01-12 15:36 Ander Conselvan de Oliveira
2015-01-12 23:34 ` Daniel Vetter
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Ander Conselvan de Oliveira @ 2015-01-12 15:36 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira, stable
Otherwise setting the rotation property will cause the primary plane to
be disabled, caused by having a 0x0 initial value.
Cc: stable@vger.kernel.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d1a4de8..fdea96c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13322,6 +13322,23 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
}
+static void primary_update_size(struct intel_crtc *crtc)
+{
+ struct intel_plane *primary = to_intel_plane(crtc->base.primary);
+
+ if (!crtc->primary_enabled)
+ return;
+
+ primary->crtc_x = 0;
+ primary->crtc_y = 0;
+ primary->crtc_w = crtc->config.pipe_src_w;
+ primary->crtc_h = crtc->config.pipe_src_h;
+ primary->src_x = 0;
+ primary->src_y = 0;
+ primary->src_w = crtc->config.pipe_src_w << 16;
+ primary->src_h = crtc->config.pipe_src_h << 16;
+}
+
static void intel_modeset_readout_hw_state(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -13332,6 +13349,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
int i;
for_each_intel_crtc(dev, crtc) {
+
memset(&crtc->config, 0, sizeof(crtc->config));
crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
@@ -13341,6 +13359,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
crtc->base.enabled = crtc->active;
crtc->primary_enabled = primary_get_hw_state(crtc);
+ primary_update_size(crtc);
DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
crtc->base.base.id,
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state
2015-01-12 15:36 [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state Ander Conselvan de Oliveira
@ 2015-01-12 23:34 ` Daniel Vetter
2015-01-13 11:24 ` Ville Syrjälä
2015-01-13 0:48 ` [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state shuang.he
2015-01-19 13:51 ` [PATCH v2] " Ander Conselvan de Oliveira
2 siblings, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2015-01-12 23:34 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx, stable
On Mon, Jan 12, 2015 at 05:36:52PM +0200, Ander Conselvan de Oliveira wrote:
> Otherwise setting the rotation property will cause the primary plane to
> be disabled, caused by having a 0x0 initial value.
>
> Cc: stable@vger.kernel.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I guess long term we need to consolidate all our plane state related
readout functions, atm it's splattered all over. But that's maybe
something for after all the atomic work has stablized a bit.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d1a4de8..fdea96c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13322,6 +13322,23 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
> return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
> }
>
> +static void primary_update_size(struct intel_crtc *crtc)
> +{
> + struct intel_plane *primary = to_intel_plane(crtc->base.primary);
> +
> + if (!crtc->primary_enabled)
> + return;
> +
> + primary->crtc_x = 0;
> + primary->crtc_y = 0;
> + primary->crtc_w = crtc->config.pipe_src_w;
> + primary->crtc_h = crtc->config.pipe_src_h;
> + primary->src_x = 0;
> + primary->src_y = 0;
> + primary->src_w = crtc->config.pipe_src_w << 16;
> + primary->src_h = crtc->config.pipe_src_h << 16;
> +}
> +
> static void intel_modeset_readout_hw_state(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -13332,6 +13349,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> int i;
>
> for_each_intel_crtc(dev, crtc) {
> +
> memset(&crtc->config, 0, sizeof(crtc->config));
>
> crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
> @@ -13341,6 +13359,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>
> crtc->base.enabled = crtc->active;
> crtc->primary_enabled = primary_get_hw_state(crtc);
> + primary_update_size(crtc);
>
> DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> crtc->base.base.id,
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state
2015-01-12 15:36 [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state Ander Conselvan de Oliveira
2015-01-12 23:34 ` Daniel Vetter
@ 2015-01-13 0:48 ` shuang.he
2015-01-13 8:17 ` Jani Nikula
2015-01-19 13:51 ` [PATCH v2] " Ander Conselvan de Oliveira
2 siblings, 1 reply; 17+ messages in thread
From: shuang.he @ 2015-01-13 0:48 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, ander.conselvan.de.oliveira
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV -1 354/354 353/354
ILK 201/201 201/201
SNB +1-1 401/424 401/424
IVB -2 488/488 486/488
BYT 278/278 278/278
HSW -42 529/529 487/529
BDW -1 405/405 404/405
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*PNV igt_gem_concurrent_blit_cpu-rcs-overwrite-source-interruptible PASS(2, M7) NO_RESULT(1, M7)
SNB igt_kms_flip_flip-vs-dpms-off-vs-modeset-interruptible NSPT(1, M35)PASS(6, M35M22) PASS(1, M35)
*SNB igt_gem_concurrent_blit_gtt-rcs-early-read-interruptible PASS(7, M35M22) DMESG_WARN(1, M35)
*IVB igt_kms_plane_plane-panning-top-left-pipe-C-plane-2 PASS(2, M21M4) DMESG_WARN(1, M4)
*IVB igt_kms_plane_plane-position-hole-pipe-C-plane-1 PASS(2, M21M4) DMESG_WARN(1, M4)
HSW igt_kms_cursor_crc_cursor-size-change NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_kms_fence_pin_leak NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_kms_flip_event_leak NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_lpsp_non-edp NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_cursor NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_cursor-dpms NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_dpms-mode-unset-non-lpsp NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_dpms-non-lpsp NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_drm-resources-equal NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_fences NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_fences-dpms NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_gem-execbuf NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_gem-mmap-cpu NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_gem-mmap-gtt NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_gem-pread NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_i2c NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_modeset-non-lpsp NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_modeset-non-lpsp-stress-no-wait NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_pci-d3-state NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_pm_rpm_rte NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
HSW igt_gem_concurrent_blit_gtt-bcs-early-read-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gtt-bcs-early-read-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gtt-bcs-gpu-read-after-write-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gtt-bcs-gpu-read-after-write-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gtt-bcs-overwrite-source-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gtt-bcs-overwrite-source-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gtt-rcs-early-read-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gtt-rcs-early-read-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gtt-rcs-gpu-read-after-write-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gtt-rcs-gpu-read-after-write-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gtt-rcs-overwrite-source-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gtt-rcs-overwrite-source-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
*HSW igt_gem_concurrent_blit_gttX-bcs-early-read-interruptible DMESG_WARN(2, M40)PASS(2, M20M19) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gttX-bcs-gpu-read-after-write-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gttX-bcs-overwrite-source-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gttX-bcs-overwrite-source-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gttX-rcs-early-read-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gttX-rcs-gpu-read-after-write-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gttX-rcs-overwrite-source-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
HSW igt_gem_concurrent_blit_gttX-rcs-overwrite-source-interruptible DMESG_WARN(2, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
*BDW igt_gem_concurrent_blit_gtt-bcs-gpu-read-after-write-interruptible PASS(6, M30M28) DMESG_WARN(1, M30)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state
2015-01-13 0:48 ` [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state shuang.he
@ 2015-01-13 8:17 ` Jani Nikula
2015-01-13 8:34 ` Ander Conselvan de Oliveira
0 siblings, 1 reply; 17+ messages in thread
From: Jani Nikula @ 2015-01-13 8:17 UTC (permalink / raw)
To: shuang.he
Ander, please see if these results make sense.
BR,
Jani.
On Tue, 13 Jan 2015, shuang.he@intel.com wrote:
> Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
> -------------------------------------Summary-------------------------------------
> Platform Delta drm-intel-nightly Series Applied
> PNV -1 354/354 353/354
> ILK 201/201 201/201
> SNB +1-1 401/424 401/424
> IVB -2 488/488 486/488
> BYT 278/278 278/278
> HSW -42 529/529 487/529
> BDW -1 405/405 404/405
> -------------------------------------Detailed-------------------------------------
> Platform Test drm-intel-nightly Series Applied
> *PNV igt_gem_concurrent_blit_cpu-rcs-overwrite-source-interruptible PASS(2, M7) NO_RESULT(1, M7)
> SNB igt_kms_flip_flip-vs-dpms-off-vs-modeset-interruptible NSPT(1, M35)PASS(6, M35M22) PASS(1, M35)
> *SNB igt_gem_concurrent_blit_gtt-rcs-early-read-interruptible PASS(7, M35M22) DMESG_WARN(1, M35)
> *IVB igt_kms_plane_plane-panning-top-left-pipe-C-plane-2 PASS(2, M21M4) DMESG_WARN(1, M4)
> *IVB igt_kms_plane_plane-position-hole-pipe-C-plane-1 PASS(2, M21M4) DMESG_WARN(1, M4)
> HSW igt_kms_cursor_crc_cursor-size-change NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_kms_fence_pin_leak NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_kms_flip_event_leak NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_lpsp_non-edp NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_cursor NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_cursor-dpms NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_dpms-mode-unset-non-lpsp NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_dpms-non-lpsp NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_drm-resources-equal NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_fences NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_fences-dpms NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_gem-execbuf NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_gem-mmap-cpu NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_gem-mmap-gtt NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_gem-pread NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_i2c NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_modeset-non-lpsp NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_modeset-non-lpsp-stress-no-wait NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_pci-d3-state NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_pm_rpm_rte NSPT(3, M40M19)PASS(1, M20) NSPT(1, M19)
> HSW igt_gem_concurrent_blit_gtt-bcs-early-read-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gtt-bcs-early-read-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gtt-bcs-gpu-read-after-write-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gtt-bcs-gpu-read-after-write-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gtt-bcs-overwrite-source-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gtt-bcs-overwrite-source-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gtt-rcs-early-read-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gtt-rcs-early-read-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gtt-rcs-gpu-read-after-write-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gtt-rcs-gpu-read-after-write-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gtt-rcs-overwrite-source-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gtt-rcs-overwrite-source-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> *HSW igt_gem_concurrent_blit_gttX-bcs-early-read-interruptible DMESG_WARN(2, M40)PASS(2, M20M19) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gttX-bcs-gpu-read-after-write-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gttX-bcs-overwrite-source-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gttX-bcs-overwrite-source-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gttX-rcs-early-read-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gttX-rcs-gpu-read-after-write-interruptible DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gttX-rcs-overwrite-source-forked DMESG_WARN(3, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> HSW igt_gem_concurrent_blit_gttX-rcs-overwrite-source-interruptible DMESG_WARN(2, M40M19)PASS(1, M20) DMESG_WARN(1, M19)
> *BDW igt_gem_concurrent_blit_gtt-bcs-gpu-read-after-write-interruptible PASS(6, M30M28) DMESG_WARN(1, M30)
> Note: You need to pay more attention to line start with '*'
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state
2015-01-13 8:17 ` Jani Nikula
@ 2015-01-13 8:34 ` Ander Conselvan de Oliveira
2015-01-13 8:43 ` He, Shuang
0 siblings, 1 reply; 17+ messages in thread
From: Ander Conselvan de Oliveira @ 2015-01-13 8:34 UTC (permalink / raw)
To: Jani Nikula, shuang.he, ethan.gao, intel-gfx
On 01/13/2015 10:17 AM, Jani Nikula wrote:
>
> Ander, please see if these results make sense.
>
> BR,
> Jani.
>
> On Tue, 13 Jan 2015, shuang.he@intel.com wrote:
>> Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
>> -------------------------------------Summary-------------------------------------
>> Platform Delta drm-intel-nightly Series Applied
>> PNV -1 354/354 353/354
>> ILK 201/201 201/201
>> SNB +1-1 401/424 401/424
>> IVB -2 488/488 486/488
>> BYT 278/278 278/278
>> HSW -42 529/529 487/529
>> BDW -1 405/405 404/405
>> -------------------------------------Detailed-------------------------------------
>> Platform Test drm-intel-nightly Series Applied
>> *PNV igt_gem_concurrent_blit_cpu-rcs-overwrite-source-interruptible PASS(2, M7) NO_RESULT(1, M7)
I'm not sure of what to make out of NO_RESULT.
>> *SNB igt_gem_concurrent_blit_gtt-rcs-early-read-interruptible PASS(7, M35M22) DMESG_WARN(1, M35)
PRTS doesn't seem to have saved the dmesg for this one.
>> *IVB igt_kms_plane_plane-panning-top-left-pipe-C-plane-2 PASS(2, M21M4) DMESG_WARN(1, M4)
<3>[ 8370.823428] [drm:intel_set_pch_fifo_underrun_reporting [i915]] *ERROR* uncleared pch fifo underrun on pch transcoder C
>> *IVB igt_kms_plane_plane-position-hole-pipe-C-plane-1 PASS(2, M21M4) DMESG_WARN(1, M4)
<3>[ 8417.687721] [drm:intel_dp_start_link_train [i915]] *ERROR* too many voltage retries, give up
<3>[ 8417.694032] [drm:intel_dp_start_link_train [i915]] *ERROR* too many voltage retries, give up
<3>[ 8417.700334] [drm:intel_dp_start_link_train [i915]] *ERROR* too many voltage retries, give up
<3>[ 8417.706640] [drm:intel_dp_start_link_train [i915]] *ERROR* too many voltage retries, give up
<3>[ 8417.712947] [drm:intel_dp_start_link_train [i915]] *ERROR* too many voltage retries, give up
<3>[ 8417.719256] [drm:intel_dp_start_link_train [i915]] *ERROR* too many voltage retries, give up
<3>[ 8417.727057] [drm:intel_dp_complete_link_train [i915]] *ERROR* failed to train DP, aborting
>> *HSW igt_gem_concurrent_blit_gttX-bcs-early-read-interruptible DMESG_WARN(2, M40)PASS(2, M20M19) DMESG_WARN(1, M19)
It seems PRTS also forgot to save the dmesg for this one.
>> *BDW igt_gem_concurrent_blit_gtt-bcs-gpu-read-after-write-interruptible PASS(6, M30M28) DMESG_WARN(1, M30)
<3>[ 7724.684295] [drm:hsw_unclaimed_reg_detect.isra.10 [i915]] *ERROR* Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.gem_concurrent_blit: starting subtest gtt-bcs-gpu-read-after-write-interruptible
Are any of those known issues?
Thanks,
Ander
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state
2015-01-13 8:34 ` Ander Conselvan de Oliveira
@ 2015-01-13 8:43 ` He, Shuang
0 siblings, 0 replies; 17+ messages in thread
From: He, Shuang @ 2015-01-13 8:43 UTC (permalink / raw)
To: 'Ander Conselvan de Oliveira', Jani Nikula, Gao, Ethan,
intel-gfx@lists.freedesktop.org
> -----Original Message-----
> From: Ander Conselvan de Oliveira [mailto:conselvan2@gmail.com]
> Sent: Tuesday, January 13, 2015 4:34 PM
> To: Jani Nikula; He, Shuang; Gao, Ethan; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Initialize primary plane src/dst coords
> when reading hw state
>
> On 01/13/2015 10:17 AM, Jani Nikula wrote:
> >
> > Ander, please see if these results make sense.
> >
> > BR,
> > Jani.
> >
> > On Tue, 13 Jan 2015, shuang.he@intel.com wrote:
> >> Tested-By: PRC QA PRTS (Patch Regression Test System Contact:
> shuang.he@intel.com)
> >> -------------------------------------Summary-------------------------------------
> >> Platform Delta drm-intel-nightly Series
> Applied
> >> PNV -1 354/354
> 353/354
> >> ILK 201/201
> 201/201
> >> SNB +1-1 401/424
> 401/424
> >> IVB -2 488/488
> 486/488
> >> BYT 278/278
> 278/278
> >> HSW -42 529/529
> 487/529
> >> BDW -1 405/405
> 404/405
> >> -------------------------------------Detailed-------------------------------------
> >> Platform Test drm-intel-nightly
> Series Applied
> >> *PNV igt_gem_concurrent_blit_cpu-rcs-overwrite-source-interruptible
> PASS(2, M7) NO_RESULT(1, M7)
>
> I'm not sure of what to make out of NO_RESULT.
[He, Shuang] It means, we have tried to run it, but it didn't return any testing result (there are many possibility in this case: Test machine hung, for example)
>
> >> *SNB igt_gem_concurrent_blit_gtt-rcs-early-read-interruptible
> PASS(7, M35M22) DMESG_WARN(1, M35)
[He, Shuang] Please use the one in *.out section which is recorded by piglit itself
"igt/gem_concurrent_blit/gtt-rcs-early-read-interruptible": {
"dmesg": "[ 7857.565015] pci_pm_runtime_suspend(): intel_runtime_suspend+0x0/0x1bc [i915] returns -11
[ 7861.527641] pci_pm_runtime_suspend(): intel_runtime_suspend+0x0/0x1bc [i915] returns -11
[ 7895.526741] pci_pm_runtime_suspend(): intel_runtime_suspend+0x0/0x1bc [i915] returns -11
[ 7897.526694] pci_pm_runtime_suspend(): intel_runtime_suspend+0x0/0x1bc [i915] returns -11",
"returncode": 0,
>
> PRTS doesn't seem to have saved the dmesg for this one.
>
> >> *IVB igt_kms_plane_plane-panning-top-left-pipe-C-plane-2 PASS(2,
> M21M4) DMESG_WARN(1, M4)
>
> <3>[ 8370.823428] [drm:intel_set_pch_fifo_underrun_reporting [i915]]
> *ERROR* uncleared pch fifo underrun on pch transcoder C
>
> >> *IVB igt_kms_plane_plane-position-hole-pipe-C-plane-1 PASS(2,
> M21M4) DMESG_WARN(1, M4)
>
> <3>[ 8417.687721] [drm:intel_dp_start_link_train [i915]] *ERROR* too many
> voltage retries, give up
> <3>[ 8417.694032] [drm:intel_dp_start_link_train [i915]] *ERROR* too many
> voltage retries, give up
> <3>[ 8417.700334] [drm:intel_dp_start_link_train [i915]] *ERROR* too many
> voltage retries, give up
> <3>[ 8417.706640] [drm:intel_dp_start_link_train [i915]] *ERROR* too many
> voltage retries, give up
> <3>[ 8417.712947] [drm:intel_dp_start_link_train [i915]] *ERROR* too many
> voltage retries, give up
> <3>[ 8417.719256] [drm:intel_dp_start_link_train [i915]] *ERROR* too many
> voltage retries, give up
> <3>[ 8417.727057] [drm:intel_dp_complete_link_train [i915]] *ERROR* failed
> to train DP, aborting
>
> >> *HSW igt_gem_concurrent_blit_gttX-bcs-early-read-interruptible
> DMESG_WARN(2, M40)PASS(2, M20M19) DMESG_WARN(1, M19)
>
> It seems PRTS also forgot to save the dmesg for this one.
[He, Shuang] "igt/gem_concurrent_blit/gttX-bcs-early-read-interruptible": {
"dmesg": "[ 8127.156781] pci_pm_runtime_suspend(): intel_runtime_suspend+0x0/0x1bc [i915] returns -11
[ 8135.168107] pci_pm_runtime_suspend(): intel_runtime_suspend+0x0/0x1bc [i915] returns -11
[ 8149.163954] pci_pm_runtime_suspend(): intel_runtime_suspend+0x0/0x1bc [i915] returns -11
[ 8151.178758] pci_pm_runtime_suspend(): intel_runtime_suspend+0x0/0x1bc [i915] returns -11",
Thanks
--Shuang
>
> >> *BDW
> igt_gem_concurrent_blit_gtt-bcs-gpu-read-after-write-interruptible
> PASS(6, M30M28) DMESG_WARN(1, M30)
>
> <3>[ 7724.684295] [drm:hsw_unclaimed_reg_detect.isra.10 [i915]] *ERROR*
> Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this
> problem.gem_concurrent_blit: starting subtest
> gtt-bcs-gpu-read-after-write-interruptible
>
> Are any of those known issues?
>
> Thanks,
> Ander
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state
2015-01-12 23:34 ` Daniel Vetter
@ 2015-01-13 11:24 ` Ville Syrjälä
2015-01-13 22:23 ` Daniel Vetter
0 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2015-01-13 11:24 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Ander Conselvan de Oliveira, intel-gfx, stable
On Tue, Jan 13, 2015 at 12:34:06AM +0100, Daniel Vetter wrote:
> On Mon, Jan 12, 2015 at 05:36:52PM +0200, Ander Conselvan de Oliveira wrote:
> > Otherwise setting the rotation property will cause the primary plane to
> > be disabled, caused by having a 0x0 initial value.
> >
> > Cc: stable@vger.kernel.org
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> I guess long term we need to consolidate all our plane state related
> readout functions, atm it's splattered all over. But that's maybe
> something for after all the atomic work has stablized a bit.
We would also need to consider the user requested vs. current state
issue during resume. If someone suspends with a bunch of planes
enabled we should restore them during resume rather than overwrite
the user requested state with the current hardware state.
So the patch isn't entirely correct in that sense, but given that
we overwrite these values again based on the crtc->x/y and crtc->mode
when we restore the mode, so it should come out ok in this case. I
suppose we don't yet track the current vs. user state sufficiently
to do things in an entirely correct way.
> -Daniel
>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d1a4de8..fdea96c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13322,6 +13322,23 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
> > return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
> > }
> >
> > +static void primary_update_size(struct intel_crtc *crtc)
> > +{
> > + struct intel_plane *primary = to_intel_plane(crtc->base.primary);
> > +
> > + if (!crtc->primary_enabled)
> > + return;
> > +
> > + primary->crtc_x = 0;
> > + primary->crtc_y = 0;
> > + primary->crtc_w = crtc->config.pipe_src_w;
> > + primary->crtc_h = crtc->config.pipe_src_h;
> > + primary->src_x = 0;
> > + primary->src_y = 0;
> > + primary->src_w = crtc->config.pipe_src_w << 16;
> > + primary->src_h = crtc->config.pipe_src_h << 16;
> > +}
> > +
> > static void intel_modeset_readout_hw_state(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -13332,6 +13349,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> > int i;
> >
> > for_each_intel_crtc(dev, crtc) {
> > +
> > memset(&crtc->config, 0, sizeof(crtc->config));
> >
> > crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
> > @@ -13341,6 +13359,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >
> > crtc->base.enabled = crtc->active;
> > crtc->primary_enabled = primary_get_hw_state(crtc);
> > + primary_update_size(crtc);
> >
> > DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> > crtc->base.base.id,
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state
2015-01-13 11:24 ` Ville Syrjälä
@ 2015-01-13 22:23 ` Daniel Vetter
2015-01-19 13:43 ` [PATCH i-g-t] kms_plane: Add test that suspends/resumes before getting crc Ander Conselvan de Oliveira
0 siblings, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2015-01-13 22:23 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Ander Conselvan de Oliveira, intel-gfx, stable
On Tue, Jan 13, 2015 at 01:24:12PM +0200, Ville Syrjälä wrote:
> On Tue, Jan 13, 2015 at 12:34:06AM +0100, Daniel Vetter wrote:
> > On Mon, Jan 12, 2015 at 05:36:52PM +0200, Ander Conselvan de Oliveira wrote:
> > > Otherwise setting the rotation property will cause the primary plane to
> > > be disabled, caused by having a 0x0 initial value.
> > >
> > > Cc: stable@vger.kernel.org
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
> > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > I guess long term we need to consolidate all our plane state related
> > readout functions, atm it's splattered all over. But that's maybe
> > something for after all the atomic work has stablized a bit.
>
> We would also need to consider the user requested vs. current state
> issue during resume. If someone suspends with a bunch of planes
> enabled we should restore them during resume rather than overwrite
> the user requested state with the current hardware state.
>
> So the patch isn't entirely correct in that sense, but given that
> we overwrite these values again based on the crtc->x/y and crtc->mode
> when we restore the mode, so it should come out ok in this case. I
> suppose we don't yet track the current vs. user state sufficiently
> to do things in an entirely correct way.
Hm right I've missed that interaction completely. Do we have a testcase
which sets the primary planes to a specified place, does a suspend/resume
and then checks the placement with CRCs? Might need to do a full pass over
all the crc plane (primary, cursor, sprite) we have already and make sure
we have subtests against suspend for all interesting cases.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH i-g-t] kms_plane: Add test that suspends/resumes before getting crc
2015-01-13 22:23 ` Daniel Vetter
@ 2015-01-19 13:43 ` Ander Conselvan de Oliveira
2015-01-20 9:35 ` Daniel Vetter
0 siblings, 1 reply; 17+ messages in thread
From: Ander Conselvan de Oliveira @ 2015-01-19 13:43 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Ander Conselvan de Oliveira, intel-gfx
This adds a test that does a suspend/resume cycle between configuring
a plane and getting the crc value for the pipe. The intention is to
test if the user requested stated is restored properly, instead of
being clobbered by the state read out from the hardware.
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
tests/kms_plane.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 5aa58c4..c94eac0 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -32,6 +32,7 @@
#include "drmtest.h"
#include "igt_debugfs.h"
#include "igt_kms.h"
+#include "igt_aux.h"
typedef struct {
float red;
@@ -269,6 +270,7 @@ create_fb_for_mode__panning(data_t *data, drmModeModeInfo *mode,
enum {
TEST_PANNING_TOP_LEFT = 1 << 0,
TEST_PANNING_BOTTOM_RIGHT = 1 << 1,
+ TEST_SUSPEND_RESUME = 1 << 2,
};
static void
@@ -309,8 +311,13 @@ test_plane_panning_with_output(data_t *data,
igt_display_commit(&data->display);
+ if (flags & TEST_SUSPEND_RESUME)
+ igt_system_suspend_autoresume();
+
igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
+ igt_debug_wait_for_keypress("crc");
+
if (flags & TEST_PANNING_TOP_LEFT)
igt_assert(igt_crc_equal(&test.red_crc, &crc));
else
@@ -360,6 +367,11 @@ run_tests_for_pipe_plane(data_t *data, enum pipe pipe, enum igt_plane plane)
test_plane_panning(data, pipe, plane,
TEST_PANNING_BOTTOM_RIGHT);
+ igt_subtest_f("plane-panning-bottom-right-suspend-pipe-%s-plane-%d",
+ kmstest_pipe_name(pipe), plane)
+ test_plane_panning(data, pipe, plane,
+ TEST_PANNING_BOTTOM_RIGHT |
+ TEST_SUSPEND_RESUME);
}
static void
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2] drm/i915: Initialize primary plane src/dst coords when reading hw state
2015-01-12 15:36 [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state Ander Conselvan de Oliveira
2015-01-12 23:34 ` Daniel Vetter
2015-01-13 0:48 ` [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state shuang.he
@ 2015-01-19 13:51 ` Ander Conselvan de Oliveira
2015-01-19 23:45 ` shuang.he
2015-01-20 9:22 ` Daniel Vetter
2 siblings, 2 replies; 17+ messages in thread
From: Ander Conselvan de Oliveira @ 2015-01-19 13:51 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Ander Conselvan de Oliveira, intel-gfx, stable
Otherwise setting the rotation property will cause the primary plane to
be disabled, caused by having a 0x0 initial value.
v2: Rebase on top of the move to plane helpers.
Cc: stable@vger.kernel.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 91d8ada..ed70ca7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13327,6 +13327,23 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
}
+static void primary_update_size(struct intel_crtc *crtc)
+{
+ struct drm_plane_state *state = crtc->base.primary->state;
+
+ if (!crtc->primary_enabled)
+ return;
+
+ state->crtc_x = 0;
+ state->crtc_y = 0;
+ state->crtc_w = crtc->config.pipe_src_w;
+ state->crtc_h = crtc->config.pipe_src_h;
+ state->src_x = 0;
+ state->src_y = 0;
+ state->src_w = crtc->config.pipe_src_w << 16;
+ state->src_h = crtc->config.pipe_src_h << 16;
+}
+
static void intel_modeset_readout_hw_state(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -13337,6 +13354,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
int i;
for_each_intel_crtc(dev, crtc) {
+
memset(&crtc->config, 0, sizeof(crtc->config));
crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
@@ -13346,6 +13364,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
crtc->base.enabled = crtc->active;
crtc->primary_enabled = primary_get_hw_state(crtc);
+ primary_update_size(crtc);
DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
crtc->base.base.id,
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/i915: Initialize primary plane src/dst coords when reading hw state
2015-01-19 13:51 ` [PATCH v2] " Ander Conselvan de Oliveira
@ 2015-01-19 23:45 ` shuang.he
2015-01-20 9:22 ` Daniel Vetter
1 sibling, 0 replies; 17+ messages in thread
From: shuang.he @ 2015-01-19 23:45 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, ander.conselvan.de.oliveira
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5604
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 353/353 353/353
ILK 353/353 353/353
SNB 400/422 400/422
IVB 487/487 487/487
BYT 296/296 296/296
HSW +21 487/508 508/508
BDW 401/402 401/402
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
HSW igt_kms_cursor_crc_cursor-size-change NSPT(1, M19)TIMEOUT(1, M40)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_kms_fence_pin_leak NSPT(1, M19)DMESG_WARN(1, M40)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(1, M19)TIMEOUT(1, M40)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(1, M19)TIMEOUT(1, M40)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_lpsp_non-edp NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_cursor NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_cursor-dpms NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_dpms-mode-unset-non-lpsp NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_dpms-non-lpsp NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_drm-resources-equal NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_fences NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_fences-dpms NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_gem-execbuf NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_gem-mmap-cpu NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_gem-mmap-gtt NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_gem-pread NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_i2c NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_modeset-non-lpsp NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_modeset-non-lpsp-stress-no-wait NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_pci-d3-state NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
HSW igt_pm_rpm_rte NSPT(1, M19)PASS(7, M20M19M40) PASS(1, M20)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/i915: Initialize primary plane src/dst coords when reading hw state
2015-01-19 13:51 ` [PATCH v2] " Ander Conselvan de Oliveira
2015-01-19 23:45 ` shuang.he
@ 2015-01-20 9:22 ` Daniel Vetter
2015-01-20 9:32 ` Ander Conselvan de Oliveira
2015-01-20 9:33 ` Ander Conselvan de Oliveira
1 sibling, 2 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-01-20 9:22 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: stable, intel-gfx
On Mon, Jan 19, 2015 at 03:51:47PM +0200, Ander Conselvan de Oliveira wrote:
> Otherwise setting the rotation property will cause the primary plane to
> be disabled, caused by having a 0x0 initial value.
>
> v2: Rebase on top of the move to plane helpers.
>
> Cc: stable@vger.kernel.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Testcase: igt/ksm_plane/*-suspend-*
> ---
> drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 91d8ada..ed70ca7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13327,6 +13327,23 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
> return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
> }
>
> +static void primary_update_size(struct intel_crtc *crtc)
> +{
> + struct drm_plane_state *state = crtc->base.primary->state;
> +
> + if (!crtc->primary_enabled)
> + return;
> +
> + state->crtc_x = 0;
> + state->crtc_y = 0;
> + state->crtc_w = crtc->config.pipe_src_w;
> + state->crtc_h = crtc->config.pipe_src_h;
> + state->src_x = 0;
> + state->src_y = 0;
> + state->src_w = crtc->config.pipe_src_w << 16;
> + state->src_h = crtc->config.pipe_src_h << 16;
> +}
> +
> static void intel_modeset_readout_hw_state(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -13337,6 +13354,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> int i;
>
> for_each_intel_crtc(dev, crtc) {
> +
Spurious hunk.
> memset(&crtc->config, 0, sizeof(crtc->config));
>
> crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
> @@ -13346,6 +13364,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>
> crtc->base.enabled = crtc->active;
> crtc->primary_enabled = primary_get_hw_state(crtc);
> + primary_update_size(crtc);
I think Ville raised a really good point about the fragility of this and
restoring plane state correctly. I think conceptually it makes more sense
to restore the primary plane state together with the fb in the loop at the
end of intel_modeset_init. Would that still work, or is that too late for
when we change pipe state when sanititizing crtcs?
-Daniel
>
> DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> crtc->base.base.id,
> --
> 1.9.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/i915: Initialize primary plane src/dst coords when reading hw state
2015-01-20 9:22 ` Daniel Vetter
@ 2015-01-20 9:32 ` Ander Conselvan de Oliveira
2015-01-20 9:33 ` Ander Conselvan de Oliveira
1 sibling, 0 replies; 17+ messages in thread
From: Ander Conselvan de Oliveira @ 2015-01-20 9:32 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Ville Syrjälä, intel-gfx, stable
On 01/20/2015 11:22 AM, Daniel Vetter wrote:
> On Mon, Jan 19, 2015 at 03:51:47PM +0200, Ander Conselvan de Oliveira wrote:
>> Otherwise setting the rotation property will cause the primary plane to
>> be disabled, caused by having a 0x0 initial value.
>>
>> v2: Rebase on top of the move to plane helpers.
>>
>> Cc: stable@vger.kernel.org
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
>> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>
> Testcase: igt/ksm_plane/*-suspend-*
>
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 91d8ada..ed70ca7 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13327,6 +13327,23 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
>> return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
>> }
>>
>> +static void primary_update_size(struct intel_crtc *crtc)
>> +{
>> + struct drm_plane_state *state = crtc->base.primary->state;
>> +
>> + if (!crtc->primary_enabled)
>> + return;
>> +
>> + state->crtc_x = 0;
>> + state->crtc_y = 0;
>> + state->crtc_w = crtc->config.pipe_src_w;
>> + state->crtc_h = crtc->config.pipe_src_h;
>> + state->src_x = 0;
>> + state->src_y = 0;
>> + state->src_w = crtc->config.pipe_src_w << 16;
>> + state->src_h = crtc->config.pipe_src_h << 16;
>> +}
>> +
>> static void intel_modeset_readout_hw_state(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -13337,6 +13354,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>> int i;
>>
>> for_each_intel_crtc(dev, crtc) {
>> +
>
> Spurious hunk.
>
>> memset(&crtc->config, 0, sizeof(crtc->config));
>>
>> crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
>> @@ -13346,6 +13364,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>>
>> crtc->base.enabled = crtc->active;
>> crtc->primary_enabled = primary_get_hw_state(crtc);
>> + primary_update_size(crtc);
>
> I think Ville raised a really good point about the fragility of this and
> restoring plane state correctly. I think conceptually it makes more sense
> to restore the primary plane state together with the fb in the loop at the
> end of intel_modeset_init. Would that still work, or is that too late for
> when we change pipe state when sanititizing crtcs?
That should work. Actually, Chris sent a patch that did that some time
ago, and Ville commented that "[he was] thinking that for now these
would be better placed in intel_modeset_readout_hw_state() where we read
out the primary plane enabled state as well". [1]
So just to make it complete clear, does the problem of restoring plane
state correctly supersedes Ville's previous comment?
[1]
http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/46466
Ander
> -Daniel
>
>>
>> DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
>> crtc->base.base.id,
>> --
>> 1.9.1
>>
>
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/i915: Initialize primary plane src/dst coords when reading hw state
2015-01-20 9:22 ` Daniel Vetter
2015-01-20 9:32 ` Ander Conselvan de Oliveira
@ 2015-01-20 9:33 ` Ander Conselvan de Oliveira
2015-01-20 9:46 ` Ville Syrjälä
1 sibling, 1 reply; 17+ messages in thread
From: Ander Conselvan de Oliveira @ 2015-01-20 9:33 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Ville Syrjälä, intel-gfx, stable
On 01/20/2015 11:22 AM, Daniel Vetter wrote:
> On Mon, Jan 19, 2015 at 03:51:47PM +0200, Ander Conselvan de Oliveira wrote:
>> Otherwise setting the rotation property will cause the primary plane to
>> be disabled, caused by having a 0x0 initial value.
>>
>> v2: Rebase on top of the move to plane helpers.
>>
>> Cc: stable@vger.kernel.org
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
>> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>
> Testcase: igt/ksm_plane/*-suspend-*
>
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 91d8ada..ed70ca7 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13327,6 +13327,23 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
>> return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
>> }
>>
>> +static void primary_update_size(struct intel_crtc *crtc)
>> +{
>> + struct drm_plane_state *state = crtc->base.primary->state;
>> +
>> + if (!crtc->primary_enabled)
>> + return;
>> +
>> + state->crtc_x = 0;
>> + state->crtc_y = 0;
>> + state->crtc_w = crtc->config.pipe_src_w;
>> + state->crtc_h = crtc->config.pipe_src_h;
>> + state->src_x = 0;
>> + state->src_y = 0;
>> + state->src_w = crtc->config.pipe_src_w << 16;
>> + state->src_h = crtc->config.pipe_src_h << 16;
>> +}
>> +
>> static void intel_modeset_readout_hw_state(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -13337,6 +13354,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>> int i;
>>
>> for_each_intel_crtc(dev, crtc) {
>> +
>
> Spurious hunk.
>
>> memset(&crtc->config, 0, sizeof(crtc->config));
>>
>> crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
>> @@ -13346,6 +13364,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>>
>> crtc->base.enabled = crtc->active;
>> crtc->primary_enabled = primary_get_hw_state(crtc);
>> + primary_update_size(crtc);
>
> I think Ville raised a really good point about the fragility of this and
> restoring plane state correctly. I think conceptually it makes more sense
> to restore the primary plane state together with the fb in the loop at the
> end of intel_modeset_init. Would that still work, or is that too late for
> when we change pipe state when sanititizing crtcs?
That should work. Actually, Chris sent a patch that did that some time
ago, and Ville commented that "[he was] thinking that for now these
would be better placed in intel_modeset_readout_hw_state() where we read
out the primary plane enabled state as well". [1]
So just to make it completely clear, does the problem of restoring plane
state correctly supersedes Ville's previous comment?
[1]
http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/46466
Ander
> -Daniel
>
>>
>> DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
>> crtc->base.base.id,
>> --
>> 1.9.1
>>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH i-g-t] kms_plane: Add test that suspends/resumes before getting crc
2015-01-19 13:43 ` [PATCH i-g-t] kms_plane: Add test that suspends/resumes before getting crc Ander Conselvan de Oliveira
@ 2015-01-20 9:35 ` Daniel Vetter
0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-01-20 9:35 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx
On Mon, Jan 19, 2015 at 03:43:57PM +0200, Ander Conselvan de Oliveira wrote:
> This adds a test that does a suspend/resume cycle between configuring
> a plane and getting the crc value for the pipe. The intention is to
> test if the user requested stated is restored properly, instead of
> being clobbered by the state read out from the hardware.
>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Applied, thanks for the patch.
-Daniel
> ---
> tests/kms_plane.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 5aa58c4..c94eac0 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -32,6 +32,7 @@
> #include "drmtest.h"
> #include "igt_debugfs.h"
> #include "igt_kms.h"
> +#include "igt_aux.h"
>
> typedef struct {
> float red;
> @@ -269,6 +270,7 @@ create_fb_for_mode__panning(data_t *data, drmModeModeInfo *mode,
> enum {
> TEST_PANNING_TOP_LEFT = 1 << 0,
> TEST_PANNING_BOTTOM_RIGHT = 1 << 1,
> + TEST_SUSPEND_RESUME = 1 << 2,
> };
>
> static void
> @@ -309,8 +311,13 @@ test_plane_panning_with_output(data_t *data,
>
> igt_display_commit(&data->display);
>
> + if (flags & TEST_SUSPEND_RESUME)
> + igt_system_suspend_autoresume();
> +
> igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
>
> + igt_debug_wait_for_keypress("crc");
> +
> if (flags & TEST_PANNING_TOP_LEFT)
> igt_assert(igt_crc_equal(&test.red_crc, &crc));
> else
> @@ -360,6 +367,11 @@ run_tests_for_pipe_plane(data_t *data, enum pipe pipe, enum igt_plane plane)
> test_plane_panning(data, pipe, plane,
> TEST_PANNING_BOTTOM_RIGHT);
>
> + igt_subtest_f("plane-panning-bottom-right-suspend-pipe-%s-plane-%d",
> + kmstest_pipe_name(pipe), plane)
> + test_plane_panning(data, pipe, plane,
> + TEST_PANNING_BOTTOM_RIGHT |
> + TEST_SUSPEND_RESUME);
> }
>
> static void
> --
> 1.9.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/i915: Initialize primary plane src/dst coords when reading hw state
2015-01-20 9:33 ` Ander Conselvan de Oliveira
@ 2015-01-20 9:46 ` Ville Syrjälä
2015-01-20 9:56 ` Daniel Vetter
0 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2015-01-20 9:46 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx, stable
On Tue, Jan 20, 2015 at 11:33:22AM +0200, Ander Conselvan de Oliveira wrote:
> On 01/20/2015 11:22 AM, Daniel Vetter wrote:
> > On Mon, Jan 19, 2015 at 03:51:47PM +0200, Ander Conselvan de Oliveira wrote:
> >> Otherwise setting the rotation property will cause the primary plane to
> >> be disabled, caused by having a 0x0 initial value.
> >>
> >> v2: Rebase on top of the move to plane helpers.
> >>
> >> Cc: stable@vger.kernel.org
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
> >> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> >
> > Testcase: igt/ksm_plane/*-suspend-*
> >
> >> ---
> >> drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
> >> 1 file changed, 19 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 91d8ada..ed70ca7 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -13327,6 +13327,23 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
> >> return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
> >> }
> >>
> >> +static void primary_update_size(struct intel_crtc *crtc)
> >> +{
> >> + struct drm_plane_state *state = crtc->base.primary->state;
> >> +
> >> + if (!crtc->primary_enabled)
> >> + return;
> >> +
> >> + state->crtc_x = 0;
> >> + state->crtc_y = 0;
> >> + state->crtc_w = crtc->config.pipe_src_w;
> >> + state->crtc_h = crtc->config.pipe_src_h;
> >> + state->src_x = 0;
> >> + state->src_y = 0;
> >> + state->src_w = crtc->config.pipe_src_w << 16;
> >> + state->src_h = crtc->config.pipe_src_h << 16;
> >> +}
> >> +
> >> static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >> {
> >> struct drm_i915_private *dev_priv = dev->dev_private;
> >> @@ -13337,6 +13354,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >> int i;
> >>
> >> for_each_intel_crtc(dev, crtc) {
> >> +
> >
> > Spurious hunk.
> >
> >> memset(&crtc->config, 0, sizeof(crtc->config));
> >>
> >> crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
> >> @@ -13346,6 +13364,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >>
> >> crtc->base.enabled = crtc->active;
> >> crtc->primary_enabled = primary_get_hw_state(crtc);
> >> + primary_update_size(crtc);
> >
> > I think Ville raised a really good point about the fragility of this and
> > restoring plane state correctly. I think conceptually it makes more sense
> > to restore the primary plane state together with the fb in the loop at the
> > end of intel_modeset_init. Would that still work, or is that too late for
> > when we change pipe state when sanititizing crtcs?
>
> That should work. Actually, Chris sent a patch that did that some time
> ago, and Ville commented that "[he was] thinking that for now these
> would be better placed in intel_modeset_readout_hw_state() where we read
> out the primary plane enabled state as well". [1]
>
> So just to make it completely clear, does the problem of restoring plane
> state correctly supersedes Ville's previous comment?
Well, I still think intel_modeset_readout_hw_state() is the right place
for this. We would just need to keep the user state and current state
clearly separated (intel_modeset_readout_hw_state() should not touch the
user state). But if that's too hard to do without massive changes, maybe
we can put it somewhere else in the meantime.
>
> [1]
> http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/46466
>
> Ander
>
>
> > -Daniel
> >
> >>
> >> DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> >> crtc->base.base.id,
> >> --
> >> 1.9.1
> >>
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/i915: Initialize primary plane src/dst coords when reading hw state
2015-01-20 9:46 ` Ville Syrjälä
@ 2015-01-20 9:56 ` Daniel Vetter
0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-01-20 9:56 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, stable
On Tue, Jan 20, 2015 at 11:46:57AM +0200, Ville Syrjälä wrote:
> On Tue, Jan 20, 2015 at 11:33:22AM +0200, Ander Conselvan de Oliveira wrote:
> > On 01/20/2015 11:22 AM, Daniel Vetter wrote:
> > > On Mon, Jan 19, 2015 at 03:51:47PM +0200, Ander Conselvan de Oliveira wrote:
> > >> @@ -13346,6 +13364,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> > >>
> > >> crtc->base.enabled = crtc->active;
> > >> crtc->primary_enabled = primary_get_hw_state(crtc);
> > >> + primary_update_size(crtc);
> > >
> > > I think Ville raised a really good point about the fragility of this and
> > > restoring plane state correctly. I think conceptually it makes more sense
> > > to restore the primary plane state together with the fb in the loop at the
> > > end of intel_modeset_init. Would that still work, or is that too late for
> > > when we change pipe state when sanititizing crtcs?
> >
> > That should work. Actually, Chris sent a patch that did that some time
> > ago, and Ville commented that "[he was] thinking that for now these
> > would be better placed in intel_modeset_readout_hw_state() where we read
> > out the primary plane enabled state as well". [1]
> >
> > So just to make it completely clear, does the problem of restoring plane
> > state correctly supersedes Ville's previous comment?
>
> Well, I still think intel_modeset_readout_hw_state() is the right place
> for this. We would just need to keep the user state and current state
> clearly separated (intel_modeset_readout_hw_state() should not touch the
> user state). But if that's too hard to do without massive changes, maybe
> we can put it somewhere else in the meantime.
Well most of the plane state readout for the initial config (which this
seems to be a problem with) isn't in that function but in modeset_init.
And we currently have no means to check the requested vs the actual state
for the plane configuration.
Thinking about that we probably don't need that really: The reason for the
modeset config checks is to catch bugs with requested vs. actual state,
since we can't test anything really automatically. But with plane config
changes we can do full functional tests using display CRCs. So from that
pov I don't see a big reason to add all that complexity.
Long term we should try to consolidate the plane state readout a bit
(outside of readout_hw_state perhaps), but for now just adding the code in
the modeset_init loop seems best to me.
>> > [1]
> > http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/46466
In short I disagree with your comment referenced above.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-01-20 9:56 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-12 15:36 [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state Ander Conselvan de Oliveira
2015-01-12 23:34 ` Daniel Vetter
2015-01-13 11:24 ` Ville Syrjälä
2015-01-13 22:23 ` Daniel Vetter
2015-01-19 13:43 ` [PATCH i-g-t] kms_plane: Add test that suspends/resumes before getting crc Ander Conselvan de Oliveira
2015-01-20 9:35 ` Daniel Vetter
2015-01-13 0:48 ` [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state shuang.he
2015-01-13 8:17 ` Jani Nikula
2015-01-13 8:34 ` Ander Conselvan de Oliveira
2015-01-13 8:43 ` He, Shuang
2015-01-19 13:51 ` [PATCH v2] " Ander Conselvan de Oliveira
2015-01-19 23:45 ` shuang.he
2015-01-20 9:22 ` Daniel Vetter
2015-01-20 9:32 ` Ander Conselvan de Oliveira
2015-01-20 9:33 ` Ander Conselvan de Oliveira
2015-01-20 9:46 ` Ville Syrjälä
2015-01-20 9:56 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox