diff mbox

drm/i915: Don't WARN on BDW when PCH is propperly identified.

Message ID 1421698435-13617-1-git-send-email-rodrigo.vivi@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rodrigo Vivi Jan. 19, 2015, 8:13 p.m. UTC
We have BDW that PCH might not be propperly identified and we force it to be LPT.
However when we correctly identify it is LPT we incorrectly WARN that this isn't
a Haswell. For now we have many platforms out there we still need to force
LPT so let's keep the force below, but let's remove the WARN when everything is
propperly identified.

For reference we started receiving logs with this uncessary and incorrect WARN.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reference: https://bugs.freedesktop.org/attachment.cgi?id=110972
---
 drivers/gpu/drm/i915/i915_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jani Nikula Jan. 20, 2015, 8:51 a.m. UTC | #1
On Mon, 19 Jan 2015, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> We have BDW that PCH might not be propperly identified and we force it
> to be LPT.  However when we correctly identify it is LPT we
> incorrectly WARN that this isn't a Haswell. For now we have many
> platforms out there we still need to force LPT so let's keep the force
> below, but let's remove the WARN when everything is propperly
> identified.

Before this we haven't tested the BDW + non-LP LPT combination. Maybe we
should do some review of any checks against
INTEL_PCH_LPT_LP_DEVICE_ID_TYPE in BDW code. Those have always matched
before.

If the pch ids are getting identified right, shouldn't we then move
INTEL_PCH_LPT_LP_DEVICE_ID_TYPE check higher up in the pch
identification, and accept BDW for that too, and make IS_BROADWELL(dev)
a fallback after both the LPT variants have been checked?

Finally the condition below is busted!

> For reference we started receiving logs with this uncessary and incorrect WARN.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reference: https://bugs.freedesktop.org/attachment.cgi?id=110972
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 308774f..d4ed3a9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -462,7 +462,7 @@ void intel_detect_pch(struct drm_device *dev)
>  			} else if (id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
>  				dev_priv->pch_type = PCH_LPT;
>  				DRM_DEBUG_KMS("Found LynxPoint PCH\n");
> -				WARN_ON(!IS_HASWELL(dev));
> +				WARN_ON(!IS_HASWELL(dev) || !IS_BROADWELL(dev));

This will warn always...

BR,
Jani.

>  				WARN_ON(IS_HSW_ULT(dev));
>  			} else if (IS_BROADWELL(dev)) {
>  				dev_priv->pch_type = PCH_LPT;
> -- 
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Rodrigo Vivi Jan. 20, 2015, 6:17 p.m. UTC | #2
Thanks Jani.

Please ignore this patch... I'm testing the propper clean on BDWs here
and submiting another patch,

On Tue, Jan 20, 2015 at 12:51 AM, Jani Nikula
<jani.nikula@linux.intel.com> wrote:
> On Mon, 19 Jan 2015, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>> We have BDW that PCH might not be propperly identified and we force it
>> to be LPT.  However when we correctly identify it is LPT we
>> incorrectly WARN that this isn't a Haswell. For now we have many
>> platforms out there we still need to force LPT so let's keep the force
>> below, but let's remove the WARN when everything is propperly
>> identified.
>
> Before this we haven't tested the BDW + non-LP LPT combination. Maybe we
> should do some review of any checks against
> INTEL_PCH_LPT_LP_DEVICE_ID_TYPE in BDW code. Those have always matched
> before.
>
> If the pch ids are getting identified right, shouldn't we then move
> INTEL_PCH_LPT_LP_DEVICE_ID_TYPE check higher up in the pch
> identification, and accept BDW for that too, and make IS_BROADWELL(dev)
> a fallback after both the LPT variants have been checked?
>
> Finally the condition below is busted!
>
>> For reference we started receiving logs with this uncessary and incorrect WARN.
>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Reference: https://bugs.freedesktop.org/attachment.cgi?id=110972
>> ---
>>  drivers/gpu/drm/i915/i915_drv.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 308774f..d4ed3a9 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -462,7 +462,7 @@ void intel_detect_pch(struct drm_device *dev)
>>                       } else if (id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
>>                               dev_priv->pch_type = PCH_LPT;
>>                               DRM_DEBUG_KMS("Found LynxPoint PCH\n");
>> -                             WARN_ON(!IS_HASWELL(dev));
>> +                             WARN_ON(!IS_HASWELL(dev) || !IS_BROADWELL(dev));
>
> This will warn always...
>
> BR,
> Jani.
>
>>                               WARN_ON(IS_HSW_ULT(dev));
>>                       } else if (IS_BROADWELL(dev)) {
>>                               dev_priv->pch_type = PCH_LPT;
>> --
>> 2.1.0
>>
>> _______________________________________________
>> 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
Shuang He Jan. 22, 2015, 1:24 a.m. UTC | #3
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5607
-------------------------------------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(8, M20M19M40)      PASS(1, M40)
 HSW  igt_kms_fence_pin_leak      NSPT(1, M19)DMESG_WARN(1, M40)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip      NSPT(1, M19)TIMEOUT(1, M40)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip      NSPT(1, M19)TIMEOUT(1, M40)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_lpsp_non-edp      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_cursor      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_cursor-dpms      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_dpms-mode-unset-non-lpsp      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_dpms-non-lpsp      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_drm-resources-equal      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_fences      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_fences-dpms      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_gem-execbuf      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_gem-mmap-cpu      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_gem-mmap-gtt      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_gem-pread      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_i2c      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_modeset-non-lpsp      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_modeset-non-lpsp-stress-no-wait      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_pci-d3-state      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
 HSW  igt_pm_rpm_rte      NSPT(1, M19)PASS(8, M20M19M40)      PASS(1, M40)
Note: You need to pay more attention to line start with '*'
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 308774f..d4ed3a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -462,7 +462,7 @@  void intel_detect_pch(struct drm_device *dev)
 			} else if (id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
 				dev_priv->pch_type = PCH_LPT;
 				DRM_DEBUG_KMS("Found LynxPoint PCH\n");
-				WARN_ON(!IS_HASWELL(dev));
+				WARN_ON(!IS_HASWELL(dev) || !IS_BROADWELL(dev));
 				WARN_ON(IS_HSW_ULT(dev));
 			} else if (IS_BROADWELL(dev)) {
 				dev_priv->pch_type = PCH_LPT;