diff mbox

drm/i915: Initialize primary plane src/dst coords when reading hw state

Message ID 1421077012-25369-1-git-send-email-ander.conselvan.de.oliveira@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ander Conselvan de Oliveira Jan. 12, 2015, 3:36 p.m. UTC 
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(+)

Comments

Daniel Vetter Jan. 12, 2015, 11:34 p.m. UTC | #1 
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
Shuang He Jan. 13, 2015, 12:48 a.m. UTC | #2 
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 '*'
Jani Nikula Jan. 13, 2015, 8:17 a.m. UTC | #3 
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
Ander Conselvan de Oliveira Jan. 13, 2015, 8:34 a.m. UTC | #4 
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
Shuang He Jan. 13, 2015, 8:43 a.m. UTC | #5 
> -----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
Ville Syrjala Jan. 13, 2015, 11:24 a.m. UTC | #6 
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
Daniel Vetter Jan. 13, 2015, 10:23 p.m. UTC | #7 
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
diff mbox

Patch

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,