public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: split off Pineview desktop and mobile device info
@ 2016-12-07 22:10 Jani Nikula
  2016-12-07 22:10 ` [PATCH 2/2] drm/i915: reduce PCI ID duplication in IS_IRONLAKE_M() Jani Nikula
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Jani Nikula @ 2016-12-07 22:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

This lets us use IS_MOBILE() for distinguishing the desktop and mobile
parts instead of duplicating PCI IDs in Pineview specific checks.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 arch/x86/kernel/early-quirks.c  |  3 ++-
 drivers/gpu/drm/i915/i915_drv.h |  2 --
 drivers/gpu/drm/i915/i915_pci.c | 12 ++++++++++--
 drivers/gpu/drm/i915/intel_pm.c |  4 ++--
 include/drm/i915_pciids.h       |  6 ++++--
 5 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index 6a08e25a48d8..34af418d88cc 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -508,7 +508,8 @@ static const struct pci_device_id intel_early_ids[] __initconst = {
 	INTEL_I945G_IDS(&gen3_early_ops),
 	INTEL_I945GM_IDS(&gen3_early_ops),
 	INTEL_VLV_IDS(&gen6_early_ops),
-	INTEL_PINEVIEW_IDS(&gen3_early_ops),
+	INTEL_PINEVIEW_D_IDS(&gen3_early_ops),
+	INTEL_PINEVIEW_M_IDS(&gen3_early_ops),
 	INTEL_I965G_IDS(&gen3_early_ops),
 	INTEL_G33_IDS(&gen3_early_ops),
 	INTEL_I965GM_IDS(&gen3_early_ops),
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1480e733312a..ee1726b28b82 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2616,8 +2616,6 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define IS_G45(dev_priv)	((dev_priv)->info.platform == INTEL_G45)
 #define IS_GM45(dev_priv)	((dev_priv)->info.platform == INTEL_GM45)
 #define IS_G4X(dev_priv)	(IS_G45(dev_priv) || IS_GM45(dev_priv))
-#define IS_PINEVIEW_G(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa001)
-#define IS_PINEVIEW_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa011)
 #define IS_PINEVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_PINEVIEW)
 #define IS_G33(dev_priv)	((dev_priv)->info.platform == INTEL_G33)
 #define IS_IRONLAKE_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0046)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 93f50ef2a309..451113f79499 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -139,7 +139,14 @@ static const struct intel_device_info intel_g33_info = {
 	.has_overlay = 1,
 };
 
-static const struct intel_device_info intel_pineview_info = {
+static const struct intel_device_info intel_pineview_d_info = {
+	GEN3_FEATURES,
+	.platform = INTEL_PINEVIEW,
+	.has_hotplug = 1,
+	.has_overlay = 1,
+};
+
+static const struct intel_device_info intel_pineview_m_info = {
 	GEN3_FEATURES,
 	.platform = INTEL_PINEVIEW, .is_mobile = 1,
 	.has_hotplug = 1,
@@ -444,7 +451,8 @@ static const struct pci_device_id pciidlist[] = {
 	INTEL_I965GM_IDS(&intel_i965gm_info),
 	INTEL_GM45_IDS(&intel_gm45_info),
 	INTEL_G45_IDS(&intel_g45_info),
-	INTEL_PINEVIEW_IDS(&intel_pineview_info),
+	INTEL_PINEVIEW_D_IDS(&intel_pineview_d_info),
+	INTEL_PINEVIEW_M_IDS(&intel_pineview_m_info),
 	INTEL_IRONLAKE_D_IDS(&intel_ironlake_d_info),
 	INTEL_IRONLAKE_M_IDS(&intel_ironlake_m_info),
 	INTEL_SNB_D_IDS(&intel_sandybridge_d_info),
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9171431558a3..afe07947e51c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -659,7 +659,7 @@ static void pineview_update_wm(struct intel_crtc *unused_crtc)
 	u32 reg;
 	unsigned long wm;
 
-	latency = intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
+	latency = intel_get_cxsr_latency(!IS_MOBILE(dev_priv),
 					 dev_priv->is_ddr3,
 					 dev_priv->fsb_freq,
 					 dev_priv->mem_freq);
@@ -7730,7 +7730,7 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
 		vlv_setup_wm_latency(dev_priv);
 		dev_priv->display.update_wm = vlv_update_wm;
 	} else if (IS_PINEVIEW(dev_priv)) {
-		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
+		if (!intel_get_cxsr_latency(!IS_MOBILE(dev_priv),
 					    dev_priv->is_ddr3,
 					    dev_priv->fsb_freq,
 					    dev_priv->mem_freq)) {
diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index 540be9ff0346..9c226eb1788f 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -100,8 +100,10 @@
 	INTEL_VGA_DEVICE(0x2e42, info), /* B43_G */ \
 	INTEL_VGA_DEVICE(0x2e92, info)	/* B43_G.1 */
 
-#define INTEL_PINEVIEW_IDS(info)			\
-	INTEL_VGA_DEVICE(0xa001, info),			\
+#define INTEL_PINEVIEW_D_IDS(info) \
+	INTEL_VGA_DEVICE(0xa001, info)
+
+#define INTEL_PINEVIEW_M_IDS(info) \
 	INTEL_VGA_DEVICE(0xa011, info)
 
 #define INTEL_IRONLAKE_D_IDS(info) \
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] drm/i915: reduce PCI ID duplication in IS_IRONLAKE_M()
  2016-12-07 22:10 [PATCH 1/2] drm/i915: split off Pineview desktop and mobile device info Jani Nikula
@ 2016-12-07 22:10 ` Jani Nikula
  2016-12-07 22:19 ` [PATCH 1/2] drm/i915: split off Pineview desktop and mobile device info Jani Nikula
  2016-12-07 23:45 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
  2 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2016-12-07 22:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Reduce PCI ID duplication by replacing direct PCI ID check with
INTEL_IRONLAKE && IS_MOBILE() check.

It's slightly less optimal, but clarity and single point of truth win.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ee1726b28b82..a15161007109 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2618,7 +2618,8 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define IS_G4X(dev_priv)	(IS_G45(dev_priv) || IS_GM45(dev_priv))
 #define IS_PINEVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_PINEVIEW)
 #define IS_G33(dev_priv)	((dev_priv)->info.platform == INTEL_G33)
-#define IS_IRONLAKE_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0046)
+#define IS_IRONLAKE_M(dev_priv)	((dev_priv)->info.platform == INTEL_IRONLAKE && \
+				 IS_MOBILE(dev_priv))
 #define IS_IVYBRIDGE(dev_priv)	((dev_priv)->info.platform == INTEL_IVYBRIDGE)
 #define IS_IVB_GT1(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0156 || \
 				 INTEL_DEVID(dev_priv) == 0x0152 || \
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm/i915: split off Pineview desktop and mobile device info
  2016-12-07 22:10 [PATCH 1/2] drm/i915: split off Pineview desktop and mobile device info Jani Nikula
  2016-12-07 22:10 ` [PATCH 2/2] drm/i915: reduce PCI ID duplication in IS_IRONLAKE_M() Jani Nikula
@ 2016-12-07 22:19 ` Jani Nikula
  2016-12-08  9:11   ` Jani Nikula
  2016-12-07 23:45 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
  2 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2016-12-07 22:19 UTC (permalink / raw)
  To: intel-gfx

On Thu, 08 Dec 2016, Jani Nikula <jani.nikula@intel.com> wrote:
> This lets us use IS_MOBILE() for distinguishing the desktop and mobile
> parts instead of duplicating PCI IDs in Pineview specific checks.

This probably needs more thorough review of the !IS_MOBILE() paths in
parts not touched by this patch.

For example this one in i915_gem_detect_bit_6_swizzle() which was pretty
weird already before the Pineview device info patches:

	} else if (IS_MOBILE(dev_priv) ||
		   (IS_GEN3(dev_priv) &&
		    !IS_G33(dev_priv) && !IS_PINEVIEW(dev_priv))) {

BR,
Jani.


>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  arch/x86/kernel/early-quirks.c  |  3 ++-
>  drivers/gpu/drm/i915/i915_drv.h |  2 --
>  drivers/gpu/drm/i915/i915_pci.c | 12 ++++++++++--
>  drivers/gpu/drm/i915/intel_pm.c |  4 ++--
>  include/drm/i915_pciids.h       |  6 ++++--
>  5 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> index 6a08e25a48d8..34af418d88cc 100644
> --- a/arch/x86/kernel/early-quirks.c
> +++ b/arch/x86/kernel/early-quirks.c
> @@ -508,7 +508,8 @@ static const struct pci_device_id intel_early_ids[] __initconst = {
>  	INTEL_I945G_IDS(&gen3_early_ops),
>  	INTEL_I945GM_IDS(&gen3_early_ops),
>  	INTEL_VLV_IDS(&gen6_early_ops),
> -	INTEL_PINEVIEW_IDS(&gen3_early_ops),
> +	INTEL_PINEVIEW_D_IDS(&gen3_early_ops),
> +	INTEL_PINEVIEW_M_IDS(&gen3_early_ops),
>  	INTEL_I965G_IDS(&gen3_early_ops),
>  	INTEL_G33_IDS(&gen3_early_ops),
>  	INTEL_I965GM_IDS(&gen3_early_ops),
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1480e733312a..ee1726b28b82 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2616,8 +2616,6 @@ intel_info(const struct drm_i915_private *dev_priv)
>  #define IS_G45(dev_priv)	((dev_priv)->info.platform == INTEL_G45)
>  #define IS_GM45(dev_priv)	((dev_priv)->info.platform == INTEL_GM45)
>  #define IS_G4X(dev_priv)	(IS_G45(dev_priv) || IS_GM45(dev_priv))
> -#define IS_PINEVIEW_G(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa001)
> -#define IS_PINEVIEW_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa011)
>  #define IS_PINEVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_PINEVIEW)
>  #define IS_G33(dev_priv)	((dev_priv)->info.platform == INTEL_G33)
>  #define IS_IRONLAKE_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0046)
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 93f50ef2a309..451113f79499 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -139,7 +139,14 @@ static const struct intel_device_info intel_g33_info = {
>  	.has_overlay = 1,
>  };
>  
> -static const struct intel_device_info intel_pineview_info = {
> +static const struct intel_device_info intel_pineview_d_info = {
> +	GEN3_FEATURES,
> +	.platform = INTEL_PINEVIEW,
> +	.has_hotplug = 1,
> +	.has_overlay = 1,
> +};
> +
> +static const struct intel_device_info intel_pineview_m_info = {
>  	GEN3_FEATURES,
>  	.platform = INTEL_PINEVIEW, .is_mobile = 1,
>  	.has_hotplug = 1,
> @@ -444,7 +451,8 @@ static const struct pci_device_id pciidlist[] = {
>  	INTEL_I965GM_IDS(&intel_i965gm_info),
>  	INTEL_GM45_IDS(&intel_gm45_info),
>  	INTEL_G45_IDS(&intel_g45_info),
> -	INTEL_PINEVIEW_IDS(&intel_pineview_info),
> +	INTEL_PINEVIEW_D_IDS(&intel_pineview_d_info),
> +	INTEL_PINEVIEW_M_IDS(&intel_pineview_m_info),
>  	INTEL_IRONLAKE_D_IDS(&intel_ironlake_d_info),
>  	INTEL_IRONLAKE_M_IDS(&intel_ironlake_m_info),
>  	INTEL_SNB_D_IDS(&intel_sandybridge_d_info),
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 9171431558a3..afe07947e51c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -659,7 +659,7 @@ static void pineview_update_wm(struct intel_crtc *unused_crtc)
>  	u32 reg;
>  	unsigned long wm;
>  
> -	latency = intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
> +	latency = intel_get_cxsr_latency(!IS_MOBILE(dev_priv),
>  					 dev_priv->is_ddr3,
>  					 dev_priv->fsb_freq,
>  					 dev_priv->mem_freq);
> @@ -7730,7 +7730,7 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
>  		vlv_setup_wm_latency(dev_priv);
>  		dev_priv->display.update_wm = vlv_update_wm;
>  	} else if (IS_PINEVIEW(dev_priv)) {
> -		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
> +		if (!intel_get_cxsr_latency(!IS_MOBILE(dev_priv),
>  					    dev_priv->is_ddr3,
>  					    dev_priv->fsb_freq,
>  					    dev_priv->mem_freq)) {
> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> index 540be9ff0346..9c226eb1788f 100644
> --- a/include/drm/i915_pciids.h
> +++ b/include/drm/i915_pciids.h
> @@ -100,8 +100,10 @@
>  	INTEL_VGA_DEVICE(0x2e42, info), /* B43_G */ \
>  	INTEL_VGA_DEVICE(0x2e92, info)	/* B43_G.1 */
>  
> -#define INTEL_PINEVIEW_IDS(info)			\
> -	INTEL_VGA_DEVICE(0xa001, info),			\
> +#define INTEL_PINEVIEW_D_IDS(info) \
> +	INTEL_VGA_DEVICE(0xa001, info)
> +
> +#define INTEL_PINEVIEW_M_IDS(info) \
>  	INTEL_VGA_DEVICE(0xa011, info)
>  
>  #define INTEL_IRONLAKE_D_IDS(info) \

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: split off Pineview desktop and mobile device info
  2016-12-07 22:10 [PATCH 1/2] drm/i915: split off Pineview desktop and mobile device info Jani Nikula
  2016-12-07 22:10 ` [PATCH 2/2] drm/i915: reduce PCI ID duplication in IS_IRONLAKE_M() Jani Nikula
  2016-12-07 22:19 ` [PATCH 1/2] drm/i915: split off Pineview desktop and mobile device info Jani Nikula
@ 2016-12-07 23:45 ` Patchwork
  2016-12-08  6:53   ` Saarinen, Jani
  2 siblings, 1 reply; 9+ messages in thread
From: Patchwork @ 2016-12-07 23:45 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: split off Pineview desktop and mobile device info
URL   : https://patchwork.freedesktop.org/series/16511/
State : warning

== Summary ==

Series 16511v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/16511/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6770hq)

fi-bdw-5557u     total:247  pass:219  dwarn:0   dfail:0   fail:0   skip:28 
fi-bsw-n3050     total:247  pass:195  dwarn:0   dfail:0   fail:0   skip:52 
fi-bxt-t5700     total:247  pass:206  dwarn:0   dfail:0   fail:0   skip:41 
fi-byt-j1900     total:247  pass:206  dwarn:0   dfail:0   fail:0   skip:41 
fi-byt-n2820     total:247  pass:202  dwarn:0   dfail:0   fail:0   skip:45 
fi-hsw-4770      total:247  pass:214  dwarn:0   dfail:0   fail:0   skip:33 
fi-hsw-4770r     total:247  pass:214  dwarn:0   dfail:0   fail:0   skip:33 
fi-ilk-650       total:247  pass:181  dwarn:0   dfail:0   fail:0   skip:66 
fi-ivb-3520m     total:247  pass:213  dwarn:0   dfail:0   fail:0   skip:34 
fi-ivb-3770      total:247  pass:212  dwarn:0   dfail:0   fail:0   skip:35 
fi-kbl-7500u     total:247  pass:212  dwarn:0   dfail:0   fail:0   skip:35 
fi-skl-6260u     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-skl-6700hq    total:247  pass:214  dwarn:0   dfail:0   fail:0   skip:33 
fi-skl-6700k     total:247  pass:210  dwarn:3   dfail:0   fail:0   skip:34 
fi-skl-6770hq    total:247  pass:219  dwarn:1   dfail:0   fail:0   skip:27 
fi-snb-2520m     total:247  pass:202  dwarn:0   dfail:0   fail:0   skip:45 
fi-snb-2600      total:247  pass:201  dwarn:0   dfail:0   fail:0   skip:46 

ffcd96e45b3b3e4ca57d4879405d02b7b1b58946 drm-tip: 2016y-12m-07d-21h-45m-40s UTC integration manifest
9750e7f drm/i915: reduce PCI ID duplication in IS_IRONLAKE_M()
bb0d903 drm/i915: split off Pineview desktop and mobile device info

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3229/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: split off Pineview desktop and mobile device info
  2016-12-07 23:45 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
@ 2016-12-08  6:53   ` Saarinen, Jani
  0 siblings, 0 replies; 9+ messages in thread
From: Saarinen, Jani @ 2016-12-08  6:53 UTC (permalink / raw)
  To: intel-gfx@lists.freedesktop.org, Nikula, Jani

> == Series Details ==
> 
> Series: series starting with [1/2] drm/i915: split off Pineview desktop and
> mobile device info
> URL   : https://patchwork.freedesktop.org/series/16511/
> State : warning
> 
> == Summary ==
> 
> Series 16511v1 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/16511/revisions/1/mbox/
> 
> Test kms_pipe_crc_basic:
>         Subgroup suspend-read-crc-pipe-a:
>                 pass       -> DMESG-WARN (fi-skl-6770hq)
Still around: https://bugs.freedesktop.org/show_bug.cgi?id=97929

> 
> fi-bdw-5557u     total:247  pass:219  dwarn:0   dfail:0   fail:0   skip:28
> fi-bsw-n3050     total:247  pass:195  dwarn:0   dfail:0   fail:0   skip:52
> fi-bxt-t5700     total:247  pass:206  dwarn:0   dfail:0   fail:0   skip:41
> fi-byt-j1900     total:247  pass:206  dwarn:0   dfail:0   fail:0   skip:41
> fi-byt-n2820     total:247  pass:202  dwarn:0   dfail:0   fail:0   skip:45
> fi-hsw-4770      total:247  pass:214  dwarn:0   dfail:0   fail:0   skip:33
> fi-hsw-4770r     total:247  pass:214  dwarn:0   dfail:0   fail:0   skip:33
> fi-ilk-650       total:247  pass:181  dwarn:0   dfail:0   fail:0   skip:66
> fi-ivb-3520m     total:247  pass:213  dwarn:0   dfail:0   fail:0   skip:34
> fi-ivb-3770      total:247  pass:212  dwarn:0   dfail:0   fail:0   skip:35
> fi-kbl-7500u     total:247  pass:212  dwarn:0   dfail:0   fail:0   skip:35
> fi-skl-6260u     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27
> fi-skl-6700hq    total:247  pass:214  dwarn:0   dfail:0   fail:0   skip:33
> fi-skl-6700k     total:247  pass:210  dwarn:3   dfail:0   fail:0   skip:34
> fi-skl-6770hq    total:247  pass:219  dwarn:1   dfail:0   fail:0   skip:27
> fi-snb-2520m     total:247  pass:202  dwarn:0   dfail:0   fail:0   skip:45
> fi-snb-2600      total:247  pass:201  dwarn:0   dfail:0   fail:0   skip:46
> 
> ffcd96e45b3b3e4ca57d4879405d02b7b1b58946 drm-tip: 2016y-12m-07d-21h-
> 45m-40s UTC integration manifest 9750e7f drm/i915: reduce PCI ID duplication
> in IS_IRONLAKE_M()
> bb0d903 drm/i915: split off Pineview desktop and mobile device info
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3229/


Jani Saarinen
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo



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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm/i915: split off Pineview desktop and mobile device info
  2016-12-07 22:19 ` [PATCH 1/2] drm/i915: split off Pineview desktop and mobile device info Jani Nikula
@ 2016-12-08  9:11   ` Jani Nikula
  2016-12-08 15:58     ` Daniel Vetter
  0 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2016-12-08  9:11 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

On Thu, 08 Dec 2016, Jani Nikula <jani.nikula@intel.com> wrote:
> On Thu, 08 Dec 2016, Jani Nikula <jani.nikula@intel.com> wrote:
>> This lets us use IS_MOBILE() for distinguishing the desktop and mobile
>> parts instead of duplicating PCI IDs in Pineview specific checks.
>
> This probably needs more thorough review of the !IS_MOBILE() paths in
> parts not touched by this patch.

And Daniel says,

 11:06          danvet   j4ni, pnv is_mobile, even for the desktop version
 11:06          danvet   iirc
 11:07          danvet   because is_mobile has nothing to do with whether the 
                         chip is for laptops or not

so better just forget about these two patches I guess.


BR,
Jani.


>
> For example this one in i915_gem_detect_bit_6_swizzle() which was pretty
> weird already before the Pineview device info patches:
>
> 	} else if (IS_MOBILE(dev_priv) ||
> 		   (IS_GEN3(dev_priv) &&
> 		    !IS_G33(dev_priv) && !IS_PINEVIEW(dev_priv))) {
>
> BR,
> Jani.
>
>
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  arch/x86/kernel/early-quirks.c  |  3 ++-
>>  drivers/gpu/drm/i915/i915_drv.h |  2 --
>>  drivers/gpu/drm/i915/i915_pci.c | 12 ++++++++++--
>>  drivers/gpu/drm/i915/intel_pm.c |  4 ++--
>>  include/drm/i915_pciids.h       |  6 ++++--
>>  5 files changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
>> index 6a08e25a48d8..34af418d88cc 100644
>> --- a/arch/x86/kernel/early-quirks.c
>> +++ b/arch/x86/kernel/early-quirks.c
>> @@ -508,7 +508,8 @@ static const struct pci_device_id intel_early_ids[] __initconst = {
>>  	INTEL_I945G_IDS(&gen3_early_ops),
>>  	INTEL_I945GM_IDS(&gen3_early_ops),
>>  	INTEL_VLV_IDS(&gen6_early_ops),
>> -	INTEL_PINEVIEW_IDS(&gen3_early_ops),
>> +	INTEL_PINEVIEW_D_IDS(&gen3_early_ops),
>> +	INTEL_PINEVIEW_M_IDS(&gen3_early_ops),
>>  	INTEL_I965G_IDS(&gen3_early_ops),
>>  	INTEL_G33_IDS(&gen3_early_ops),
>>  	INTEL_I965GM_IDS(&gen3_early_ops),
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 1480e733312a..ee1726b28b82 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2616,8 +2616,6 @@ intel_info(const struct drm_i915_private *dev_priv)
>>  #define IS_G45(dev_priv)	((dev_priv)->info.platform == INTEL_G45)
>>  #define IS_GM45(dev_priv)	((dev_priv)->info.platform == INTEL_GM45)
>>  #define IS_G4X(dev_priv)	(IS_G45(dev_priv) || IS_GM45(dev_priv))
>> -#define IS_PINEVIEW_G(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa001)
>> -#define IS_PINEVIEW_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa011)
>>  #define IS_PINEVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_PINEVIEW)
>>  #define IS_G33(dev_priv)	((dev_priv)->info.platform == INTEL_G33)
>>  #define IS_IRONLAKE_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0046)
>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> index 93f50ef2a309..451113f79499 100644
>> --- a/drivers/gpu/drm/i915/i915_pci.c
>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> @@ -139,7 +139,14 @@ static const struct intel_device_info intel_g33_info = {
>>  	.has_overlay = 1,
>>  };
>>  
>> -static const struct intel_device_info intel_pineview_info = {
>> +static const struct intel_device_info intel_pineview_d_info = {
>> +	GEN3_FEATURES,
>> +	.platform = INTEL_PINEVIEW,
>> +	.has_hotplug = 1,
>> +	.has_overlay = 1,
>> +};
>> +
>> +static const struct intel_device_info intel_pineview_m_info = {
>>  	GEN3_FEATURES,
>>  	.platform = INTEL_PINEVIEW, .is_mobile = 1,
>>  	.has_hotplug = 1,
>> @@ -444,7 +451,8 @@ static const struct pci_device_id pciidlist[] = {
>>  	INTEL_I965GM_IDS(&intel_i965gm_info),
>>  	INTEL_GM45_IDS(&intel_gm45_info),
>>  	INTEL_G45_IDS(&intel_g45_info),
>> -	INTEL_PINEVIEW_IDS(&intel_pineview_info),
>> +	INTEL_PINEVIEW_D_IDS(&intel_pineview_d_info),
>> +	INTEL_PINEVIEW_M_IDS(&intel_pineview_m_info),
>>  	INTEL_IRONLAKE_D_IDS(&intel_ironlake_d_info),
>>  	INTEL_IRONLAKE_M_IDS(&intel_ironlake_m_info),
>>  	INTEL_SNB_D_IDS(&intel_sandybridge_d_info),
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 9171431558a3..afe07947e51c 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -659,7 +659,7 @@ static void pineview_update_wm(struct intel_crtc *unused_crtc)
>>  	u32 reg;
>>  	unsigned long wm;
>>  
>> -	latency = intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
>> +	latency = intel_get_cxsr_latency(!IS_MOBILE(dev_priv),
>>  					 dev_priv->is_ddr3,
>>  					 dev_priv->fsb_freq,
>>  					 dev_priv->mem_freq);
>> @@ -7730,7 +7730,7 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
>>  		vlv_setup_wm_latency(dev_priv);
>>  		dev_priv->display.update_wm = vlv_update_wm;
>>  	} else if (IS_PINEVIEW(dev_priv)) {
>> -		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
>> +		if (!intel_get_cxsr_latency(!IS_MOBILE(dev_priv),
>>  					    dev_priv->is_ddr3,
>>  					    dev_priv->fsb_freq,
>>  					    dev_priv->mem_freq)) {
>> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
>> index 540be9ff0346..9c226eb1788f 100644
>> --- a/include/drm/i915_pciids.h
>> +++ b/include/drm/i915_pciids.h
>> @@ -100,8 +100,10 @@
>>  	INTEL_VGA_DEVICE(0x2e42, info), /* B43_G */ \
>>  	INTEL_VGA_DEVICE(0x2e92, info)	/* B43_G.1 */
>>  
>> -#define INTEL_PINEVIEW_IDS(info)			\
>> -	INTEL_VGA_DEVICE(0xa001, info),			\
>> +#define INTEL_PINEVIEW_D_IDS(info) \
>> +	INTEL_VGA_DEVICE(0xa001, info)
>> +
>> +#define INTEL_PINEVIEW_M_IDS(info) \
>>  	INTEL_VGA_DEVICE(0xa011, info)
>>  
>>  #define INTEL_IRONLAKE_D_IDS(info) \

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm/i915: split off Pineview desktop and mobile device info
  2016-12-08  9:11   ` Jani Nikula
@ 2016-12-08 15:58     ` Daniel Vetter
  2016-12-08 16:27       ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2016-12-08 15:58 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Daniel Vetter, intel-gfx

On Thu, Dec 08, 2016 at 11:11:50AM +0200, Jani Nikula wrote:
> On Thu, 08 Dec 2016, Jani Nikula <jani.nikula@intel.com> wrote:
> > On Thu, 08 Dec 2016, Jani Nikula <jani.nikula@intel.com> wrote:
> >> This lets us use IS_MOBILE() for distinguishing the desktop and mobile
> >> parts instead of duplicating PCI IDs in Pineview specific checks.
> >
> > This probably needs more thorough review of the !IS_MOBILE() paths in
> > parts not touched by this patch.
> 
> And Daniel says,
> 
>  11:06          danvet   j4ni, pnv is_mobile, even for the desktop version
>  11:06          danvet   iirc
>  11:07          danvet   because is_mobile has nothing to do with whether the 
>                          chip is for laptops or not
> 
> so better just forget about these two patches I guess.

Yeah, IS_MOBILE is seriously confusing in our code. On gen2/3 it was
essentially matching HAS_LVDS (but we never made that one), later on the
splits started to happened on different axis (atom vs big core and stuff
like that), but somehow IS_MOBILE stuck around to this day and is
sometimes abused to restrict w/a to specific chips. If someone wants to
clean this up:

- Remove IS_MOBILE on gen5+, use specific platforms/pci ids where we need
  it.
- Rename IS_MOBILE to HAS_LVDS on gen2/3/4 (not entirely sure about gen4,
  would need to double-check that it makes sense).
- Nuke is_mobile from the chip info, it's only misleading people. Mobile
  is the future everywhere anyway ;-)

Cheers, Daniel

> 
> 
> BR,
> Jani.
> 
> 
> >
> > For example this one in i915_gem_detect_bit_6_swizzle() which was pretty
> > weird already before the Pineview device info patches:
> >
> > 	} else if (IS_MOBILE(dev_priv) ||
> > 		   (IS_GEN3(dev_priv) &&
> > 		    !IS_G33(dev_priv) && !IS_PINEVIEW(dev_priv))) {
> >
> > BR,
> > Jani.
> >
> >
> >>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  arch/x86/kernel/early-quirks.c  |  3 ++-
> >>  drivers/gpu/drm/i915/i915_drv.h |  2 --
> >>  drivers/gpu/drm/i915/i915_pci.c | 12 ++++++++++--
> >>  drivers/gpu/drm/i915/intel_pm.c |  4 ++--
> >>  include/drm/i915_pciids.h       |  6 ++++--
> >>  5 files changed, 18 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> >> index 6a08e25a48d8..34af418d88cc 100644
> >> --- a/arch/x86/kernel/early-quirks.c
> >> +++ b/arch/x86/kernel/early-quirks.c
> >> @@ -508,7 +508,8 @@ static const struct pci_device_id intel_early_ids[] __initconst = {
> >>  	INTEL_I945G_IDS(&gen3_early_ops),
> >>  	INTEL_I945GM_IDS(&gen3_early_ops),
> >>  	INTEL_VLV_IDS(&gen6_early_ops),
> >> -	INTEL_PINEVIEW_IDS(&gen3_early_ops),
> >> +	INTEL_PINEVIEW_D_IDS(&gen3_early_ops),
> >> +	INTEL_PINEVIEW_M_IDS(&gen3_early_ops),
> >>  	INTEL_I965G_IDS(&gen3_early_ops),
> >>  	INTEL_G33_IDS(&gen3_early_ops),
> >>  	INTEL_I965GM_IDS(&gen3_early_ops),
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 1480e733312a..ee1726b28b82 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -2616,8 +2616,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> >>  #define IS_G45(dev_priv)	((dev_priv)->info.platform == INTEL_G45)
> >>  #define IS_GM45(dev_priv)	((dev_priv)->info.platform == INTEL_GM45)
> >>  #define IS_G4X(dev_priv)	(IS_G45(dev_priv) || IS_GM45(dev_priv))
> >> -#define IS_PINEVIEW_G(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa001)
> >> -#define IS_PINEVIEW_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa011)
> >>  #define IS_PINEVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_PINEVIEW)
> >>  #define IS_G33(dev_priv)	((dev_priv)->info.platform == INTEL_G33)
> >>  #define IS_IRONLAKE_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0046)
> >> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> >> index 93f50ef2a309..451113f79499 100644
> >> --- a/drivers/gpu/drm/i915/i915_pci.c
> >> +++ b/drivers/gpu/drm/i915/i915_pci.c
> >> @@ -139,7 +139,14 @@ static const struct intel_device_info intel_g33_info = {
> >>  	.has_overlay = 1,
> >>  };
> >>  
> >> -static const struct intel_device_info intel_pineview_info = {
> >> +static const struct intel_device_info intel_pineview_d_info = {
> >> +	GEN3_FEATURES,
> >> +	.platform = INTEL_PINEVIEW,
> >> +	.has_hotplug = 1,
> >> +	.has_overlay = 1,
> >> +};
> >> +
> >> +static const struct intel_device_info intel_pineview_m_info = {
> >>  	GEN3_FEATURES,
> >>  	.platform = INTEL_PINEVIEW, .is_mobile = 1,
> >>  	.has_hotplug = 1,
> >> @@ -444,7 +451,8 @@ static const struct pci_device_id pciidlist[] = {
> >>  	INTEL_I965GM_IDS(&intel_i965gm_info),
> >>  	INTEL_GM45_IDS(&intel_gm45_info),
> >>  	INTEL_G45_IDS(&intel_g45_info),
> >> -	INTEL_PINEVIEW_IDS(&intel_pineview_info),
> >> +	INTEL_PINEVIEW_D_IDS(&intel_pineview_d_info),
> >> +	INTEL_PINEVIEW_M_IDS(&intel_pineview_m_info),
> >>  	INTEL_IRONLAKE_D_IDS(&intel_ironlake_d_info),
> >>  	INTEL_IRONLAKE_M_IDS(&intel_ironlake_m_info),
> >>  	INTEL_SNB_D_IDS(&intel_sandybridge_d_info),
> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> index 9171431558a3..afe07947e51c 100644
> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> @@ -659,7 +659,7 @@ static void pineview_update_wm(struct intel_crtc *unused_crtc)
> >>  	u32 reg;
> >>  	unsigned long wm;
> >>  
> >> -	latency = intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
> >> +	latency = intel_get_cxsr_latency(!IS_MOBILE(dev_priv),
> >>  					 dev_priv->is_ddr3,
> >>  					 dev_priv->fsb_freq,
> >>  					 dev_priv->mem_freq);
> >> @@ -7730,7 +7730,7 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
> >>  		vlv_setup_wm_latency(dev_priv);
> >>  		dev_priv->display.update_wm = vlv_update_wm;
> >>  	} else if (IS_PINEVIEW(dev_priv)) {
> >> -		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
> >> +		if (!intel_get_cxsr_latency(!IS_MOBILE(dev_priv),
> >>  					    dev_priv->is_ddr3,
> >>  					    dev_priv->fsb_freq,
> >>  					    dev_priv->mem_freq)) {
> >> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> >> index 540be9ff0346..9c226eb1788f 100644
> >> --- a/include/drm/i915_pciids.h
> >> +++ b/include/drm/i915_pciids.h
> >> @@ -100,8 +100,10 @@
> >>  	INTEL_VGA_DEVICE(0x2e42, info), /* B43_G */ \
> >>  	INTEL_VGA_DEVICE(0x2e92, info)	/* B43_G.1 */
> >>  
> >> -#define INTEL_PINEVIEW_IDS(info)			\
> >> -	INTEL_VGA_DEVICE(0xa001, info),			\
> >> +#define INTEL_PINEVIEW_D_IDS(info) \
> >> +	INTEL_VGA_DEVICE(0xa001, info)
> >> +
> >> +#define INTEL_PINEVIEW_M_IDS(info) \
> >>  	INTEL_VGA_DEVICE(0xa011, info)
> >>  
> >>  #define INTEL_IRONLAKE_D_IDS(info) \
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm/i915: split off Pineview desktop and mobile device info
  2016-12-08 15:58     ` Daniel Vetter
@ 2016-12-08 16:27       ` Ville Syrjälä
  2016-12-08 21:03         ` Jani Nikula
  0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2016-12-08 16:27 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Thu, Dec 08, 2016 at 04:58:21PM +0100, Daniel Vetter wrote:
> On Thu, Dec 08, 2016 at 11:11:50AM +0200, Jani Nikula wrote:
> > On Thu, 08 Dec 2016, Jani Nikula <jani.nikula@intel.com> wrote:
> > > On Thu, 08 Dec 2016, Jani Nikula <jani.nikula@intel.com> wrote:
> > >> This lets us use IS_MOBILE() for distinguishing the desktop and mobile
> > >> parts instead of duplicating PCI IDs in Pineview specific checks.
> > >
> > > This probably needs more thorough review of the !IS_MOBILE() paths in
> > > parts not touched by this patch.
> > 
> > And Daniel says,
> > 
> >  11:06          danvet   j4ni, pnv is_mobile, even for the desktop version
> >  11:06          danvet   iirc
> >  11:07          danvet   because is_mobile has nothing to do with whether the 
> >                          chip is for laptops or not
> > 
> > so better just forget about these two patches I guess.
> 
> Yeah, IS_MOBILE is seriously confusing in our code. On gen2/3 it was
> essentially matching HAS_LVDS (but we never made that one),

We do use it in quite a few places to differentiate between the mobile
and desktop variants of the chipset. There are some weird differences
between the two branches of the family, eg. some MCHBAR registers live
at different offsets, and swizzling stuff is different IIRC.

> later on the
> splits started to happened on different axis (atom vs big core and stuff
> like that), but somehow IS_MOBILE stuck around to this day and is
> sometimes abused to restrict w/a to specific chips. If someone wants to
> clean this up:
> 
> - Remove IS_MOBILE on gen5+, use specific platforms/pci ids where we need
>   it.

IIRC I did look into that once, and the only place where it was
used for ilk-ivb was eDP setup, as in "do we have port A or not?".
For hsw+ we don't use it at all, and IIRC don't even set it.

> - Rename IS_MOBILE to HAS_LVDS on gen2/3/4 (not entirely sure about gen4,
>   would need to double-check that it makes sense).

830 doesn't have the LVDS port, so would need a bit of special care. And
like I said. there are actual differences well outside the LVDS vs. not
thing.

If we can solve the ilk-ivb port A case, then I think we can kill
.is_mobile. For the older platforms we have separate IS_FOO and IS_FOO_M
type of things so those can be used instead. But for ilk-ivb we don't
have those, I think. Or maybe we do, not sure.

> - Nuke is_mobile from the chip info, it's only misleading people. Mobile
>   is the future everywhere anyway ;-)
> 
> Cheers, Daniel
> 
> > 
> > 
> > BR,
> > Jani.
> > 
> > 
> > >
> > > For example this one in i915_gem_detect_bit_6_swizzle() which was pretty
> > > weird already before the Pineview device info patches:
> > >
> > > 	} else if (IS_MOBILE(dev_priv) ||
> > > 		   (IS_GEN3(dev_priv) &&
> > > 		    !IS_G33(dev_priv) && !IS_PINEVIEW(dev_priv))) {
> > >
> > > BR,
> > > Jani.
> > >
> > >
> > >>
> > >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > >> ---
> > >>  arch/x86/kernel/early-quirks.c  |  3 ++-
> > >>  drivers/gpu/drm/i915/i915_drv.h |  2 --
> > >>  drivers/gpu/drm/i915/i915_pci.c | 12 ++++++++++--
> > >>  drivers/gpu/drm/i915/intel_pm.c |  4 ++--
> > >>  include/drm/i915_pciids.h       |  6 ++++--
> > >>  5 files changed, 18 insertions(+), 9 deletions(-)
> > >>
> > >> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> > >> index 6a08e25a48d8..34af418d88cc 100644
> > >> --- a/arch/x86/kernel/early-quirks.c
> > >> +++ b/arch/x86/kernel/early-quirks.c
> > >> @@ -508,7 +508,8 @@ static const struct pci_device_id intel_early_ids[] __initconst = {
> > >>  	INTEL_I945G_IDS(&gen3_early_ops),
> > >>  	INTEL_I945GM_IDS(&gen3_early_ops),
> > >>  	INTEL_VLV_IDS(&gen6_early_ops),
> > >> -	INTEL_PINEVIEW_IDS(&gen3_early_ops),
> > >> +	INTEL_PINEVIEW_D_IDS(&gen3_early_ops),
> > >> +	INTEL_PINEVIEW_M_IDS(&gen3_early_ops),
> > >>  	INTEL_I965G_IDS(&gen3_early_ops),
> > >>  	INTEL_G33_IDS(&gen3_early_ops),
> > >>  	INTEL_I965GM_IDS(&gen3_early_ops),
> > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > >> index 1480e733312a..ee1726b28b82 100644
> > >> --- a/drivers/gpu/drm/i915/i915_drv.h
> > >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> > >> @@ -2616,8 +2616,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> > >>  #define IS_G45(dev_priv)	((dev_priv)->info.platform == INTEL_G45)
> > >>  #define IS_GM45(dev_priv)	((dev_priv)->info.platform == INTEL_GM45)
> > >>  #define IS_G4X(dev_priv)	(IS_G45(dev_priv) || IS_GM45(dev_priv))
> > >> -#define IS_PINEVIEW_G(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa001)
> > >> -#define IS_PINEVIEW_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa011)
> > >>  #define IS_PINEVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_PINEVIEW)
> > >>  #define IS_G33(dev_priv)	((dev_priv)->info.platform == INTEL_G33)
> > >>  #define IS_IRONLAKE_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0046)
> > >> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> > >> index 93f50ef2a309..451113f79499 100644
> > >> --- a/drivers/gpu/drm/i915/i915_pci.c
> > >> +++ b/drivers/gpu/drm/i915/i915_pci.c
> > >> @@ -139,7 +139,14 @@ static const struct intel_device_info intel_g33_info = {
> > >>  	.has_overlay = 1,
> > >>  };
> > >>  
> > >> -static const struct intel_device_info intel_pineview_info = {
> > >> +static const struct intel_device_info intel_pineview_d_info = {
> > >> +	GEN3_FEATURES,
> > >> +	.platform = INTEL_PINEVIEW,
> > >> +	.has_hotplug = 1,
> > >> +	.has_overlay = 1,
> > >> +};
> > >> +
> > >> +static const struct intel_device_info intel_pineview_m_info = {
> > >>  	GEN3_FEATURES,
> > >>  	.platform = INTEL_PINEVIEW, .is_mobile = 1,
> > >>  	.has_hotplug = 1,
> > >> @@ -444,7 +451,8 @@ static const struct pci_device_id pciidlist[] = {
> > >>  	INTEL_I965GM_IDS(&intel_i965gm_info),
> > >>  	INTEL_GM45_IDS(&intel_gm45_info),
> > >>  	INTEL_G45_IDS(&intel_g45_info),
> > >> -	INTEL_PINEVIEW_IDS(&intel_pineview_info),
> > >> +	INTEL_PINEVIEW_D_IDS(&intel_pineview_d_info),
> > >> +	INTEL_PINEVIEW_M_IDS(&intel_pineview_m_info),
> > >>  	INTEL_IRONLAKE_D_IDS(&intel_ironlake_d_info),
> > >>  	INTEL_IRONLAKE_M_IDS(&intel_ironlake_m_info),
> > >>  	INTEL_SNB_D_IDS(&intel_sandybridge_d_info),
> > >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > >> index 9171431558a3..afe07947e51c 100644
> > >> --- a/drivers/gpu/drm/i915/intel_pm.c
> > >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> > >> @@ -659,7 +659,7 @@ static void pineview_update_wm(struct intel_crtc *unused_crtc)
> > >>  	u32 reg;
> > >>  	unsigned long wm;
> > >>  
> > >> -	latency = intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
> > >> +	latency = intel_get_cxsr_latency(!IS_MOBILE(dev_priv),
> > >>  					 dev_priv->is_ddr3,
> > >>  					 dev_priv->fsb_freq,
> > >>  					 dev_priv->mem_freq);
> > >> @@ -7730,7 +7730,7 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
> > >>  		vlv_setup_wm_latency(dev_priv);
> > >>  		dev_priv->display.update_wm = vlv_update_wm;
> > >>  	} else if (IS_PINEVIEW(dev_priv)) {
> > >> -		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
> > >> +		if (!intel_get_cxsr_latency(!IS_MOBILE(dev_priv),
> > >>  					    dev_priv->is_ddr3,
> > >>  					    dev_priv->fsb_freq,
> > >>  					    dev_priv->mem_freq)) {
> > >> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> > >> index 540be9ff0346..9c226eb1788f 100644
> > >> --- a/include/drm/i915_pciids.h
> > >> +++ b/include/drm/i915_pciids.h
> > >> @@ -100,8 +100,10 @@
> > >>  	INTEL_VGA_DEVICE(0x2e42, info), /* B43_G */ \
> > >>  	INTEL_VGA_DEVICE(0x2e92, info)	/* B43_G.1 */
> > >>  
> > >> -#define INTEL_PINEVIEW_IDS(info)			\
> > >> -	INTEL_VGA_DEVICE(0xa001, info),			\
> > >> +#define INTEL_PINEVIEW_D_IDS(info) \
> > >> +	INTEL_VGA_DEVICE(0xa001, info)
> > >> +
> > >> +#define INTEL_PINEVIEW_M_IDS(info) \
> > >>  	INTEL_VGA_DEVICE(0xa011, info)
> > >>  
> > >>  #define INTEL_IRONLAKE_D_IDS(info) \
> > 
> > -- 
> > Jani Nikula, Intel Open Source Technology Center
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm/i915: split off Pineview desktop and mobile device info
  2016-12-08 16:27       ` Ville Syrjälä
@ 2016-12-08 21:03         ` Jani Nikula
  0 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2016-12-08 21:03 UTC (permalink / raw)
  To: Ville Syrjälä, Daniel Vetter; +Cc: Daniel Vetter, intel-gfx

On Thu, 08 Dec 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Dec 08, 2016 at 04:58:21PM +0100, Daniel Vetter wrote:
>> On Thu, Dec 08, 2016 at 11:11:50AM +0200, Jani Nikula wrote:
>> > On Thu, 08 Dec 2016, Jani Nikula <jani.nikula@intel.com> wrote:
>> > > On Thu, 08 Dec 2016, Jani Nikula <jani.nikula@intel.com> wrote:
>> > >> This lets us use IS_MOBILE() for distinguishing the desktop and mobile
>> > >> parts instead of duplicating PCI IDs in Pineview specific checks.
>> > >
>> > > This probably needs more thorough review of the !IS_MOBILE() paths in
>> > > parts not touched by this patch.
>> > 
>> > And Daniel says,
>> > 
>> >  11:06          danvet   j4ni, pnv is_mobile, even for the desktop version
>> >  11:06          danvet   iirc
>> >  11:07          danvet   because is_mobile has nothing to do with whether the 
>> >                          chip is for laptops or not
>> > 
>> > so better just forget about these two patches I guess.
>> 
>> Yeah, IS_MOBILE is seriously confusing in our code. On gen2/3 it was
>> essentially matching HAS_LVDS (but we never made that one),
>
> We do use it in quite a few places to differentiate between the mobile
> and desktop variants of the chipset. There are some weird differences
> between the two branches of the family, eg. some MCHBAR registers live
> at different offsets, and swizzling stuff is different IIRC.
>
>> later on the
>> splits started to happened on different axis (atom vs big core and stuff
>> like that), but somehow IS_MOBILE stuck around to this day and is
>> sometimes abused to restrict w/a to specific chips. If someone wants to
>> clean this up:
>> 
>> - Remove IS_MOBILE on gen5+, use specific platforms/pci ids where we need
>>   it.
>
> IIRC I did look into that once, and the only place where it was
> used for ilk-ivb was eDP setup, as in "do we have port A or not?".
> For hsw+ we don't use it at all, and IIRC don't even set it.

There's IS_IRONLAKE_M() which looks at the PCI ID, but essentially it's
IS_IRONLAKE && IS_MOBILE.

I really dislike direct PCI ID checks in the IS_* macros, because they
duplicate PCI IDs from i915_pciids.h, which typically already has the
IDs grouped at the granularity we'd like (mobile vs. not in this
case). I wish we could do all the IS_*_{ULX,ULT,GT?} using the PCI ID
macros from one source.

BR,
Jani.


>
>> - Rename IS_MOBILE to HAS_LVDS on gen2/3/4 (not entirely sure about gen4,
>>   would need to double-check that it makes sense).
>
> 830 doesn't have the LVDS port, so would need a bit of special care. And
> like I said. there are actual differences well outside the LVDS vs. not
> thing.
>
> If we can solve the ilk-ivb port A case, then I think we can kill
> .is_mobile. For the older platforms we have separate IS_FOO and IS_FOO_M
> type of things so those can be used instead. But for ilk-ivb we don't
> have those, I think. Or maybe we do, not sure.
>
>> - Nuke is_mobile from the chip info, it's only misleading people. Mobile
>>   is the future everywhere anyway ;-)
>> 
>> Cheers, Daniel
>> 
>> > 
>> > 
>> > BR,
>> > Jani.
>> > 
>> > 
>> > >
>> > > For example this one in i915_gem_detect_bit_6_swizzle() which was pretty
>> > > weird already before the Pineview device info patches:
>> > >
>> > > 	} else if (IS_MOBILE(dev_priv) ||
>> > > 		   (IS_GEN3(dev_priv) &&
>> > > 		    !IS_G33(dev_priv) && !IS_PINEVIEW(dev_priv))) {
>> > >
>> > > BR,
>> > > Jani.
>> > >
>> > >
>> > >>
>> > >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> > >> ---
>> > >>  arch/x86/kernel/early-quirks.c  |  3 ++-
>> > >>  drivers/gpu/drm/i915/i915_drv.h |  2 --
>> > >>  drivers/gpu/drm/i915/i915_pci.c | 12 ++++++++++--
>> > >>  drivers/gpu/drm/i915/intel_pm.c |  4 ++--
>> > >>  include/drm/i915_pciids.h       |  6 ++++--
>> > >>  5 files changed, 18 insertions(+), 9 deletions(-)
>> > >>
>> > >> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
>> > >> index 6a08e25a48d8..34af418d88cc 100644
>> > >> --- a/arch/x86/kernel/early-quirks.c
>> > >> +++ b/arch/x86/kernel/early-quirks.c
>> > >> @@ -508,7 +508,8 @@ static const struct pci_device_id intel_early_ids[] __initconst = {
>> > >>  	INTEL_I945G_IDS(&gen3_early_ops),
>> > >>  	INTEL_I945GM_IDS(&gen3_early_ops),
>> > >>  	INTEL_VLV_IDS(&gen6_early_ops),
>> > >> -	INTEL_PINEVIEW_IDS(&gen3_early_ops),
>> > >> +	INTEL_PINEVIEW_D_IDS(&gen3_early_ops),
>> > >> +	INTEL_PINEVIEW_M_IDS(&gen3_early_ops),
>> > >>  	INTEL_I965G_IDS(&gen3_early_ops),
>> > >>  	INTEL_G33_IDS(&gen3_early_ops),
>> > >>  	INTEL_I965GM_IDS(&gen3_early_ops),
>> > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> > >> index 1480e733312a..ee1726b28b82 100644
>> > >> --- a/drivers/gpu/drm/i915/i915_drv.h
>> > >> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > >> @@ -2616,8 +2616,6 @@ intel_info(const struct drm_i915_private *dev_priv)
>> > >>  #define IS_G45(dev_priv)	((dev_priv)->info.platform == INTEL_G45)
>> > >>  #define IS_GM45(dev_priv)	((dev_priv)->info.platform == INTEL_GM45)
>> > >>  #define IS_G4X(dev_priv)	(IS_G45(dev_priv) || IS_GM45(dev_priv))
>> > >> -#define IS_PINEVIEW_G(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa001)
>> > >> -#define IS_PINEVIEW_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa011)
>> > >>  #define IS_PINEVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_PINEVIEW)
>> > >>  #define IS_G33(dev_priv)	((dev_priv)->info.platform == INTEL_G33)
>> > >>  #define IS_IRONLAKE_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0046)
>> > >> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> > >> index 93f50ef2a309..451113f79499 100644
>> > >> --- a/drivers/gpu/drm/i915/i915_pci.c
>> > >> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> > >> @@ -139,7 +139,14 @@ static const struct intel_device_info intel_g33_info = {
>> > >>  	.has_overlay = 1,
>> > >>  };
>> > >>  
>> > >> -static const struct intel_device_info intel_pineview_info = {
>> > >> +static const struct intel_device_info intel_pineview_d_info = {
>> > >> +	GEN3_FEATURES,
>> > >> +	.platform = INTEL_PINEVIEW,
>> > >> +	.has_hotplug = 1,
>> > >> +	.has_overlay = 1,
>> > >> +};
>> > >> +
>> > >> +static const struct intel_device_info intel_pineview_m_info = {
>> > >>  	GEN3_FEATURES,
>> > >>  	.platform = INTEL_PINEVIEW, .is_mobile = 1,
>> > >>  	.has_hotplug = 1,
>> > >> @@ -444,7 +451,8 @@ static const struct pci_device_id pciidlist[] = {
>> > >>  	INTEL_I965GM_IDS(&intel_i965gm_info),
>> > >>  	INTEL_GM45_IDS(&intel_gm45_info),
>> > >>  	INTEL_G45_IDS(&intel_g45_info),
>> > >> -	INTEL_PINEVIEW_IDS(&intel_pineview_info),
>> > >> +	INTEL_PINEVIEW_D_IDS(&intel_pineview_d_info),
>> > >> +	INTEL_PINEVIEW_M_IDS(&intel_pineview_m_info),
>> > >>  	INTEL_IRONLAKE_D_IDS(&intel_ironlake_d_info),
>> > >>  	INTEL_IRONLAKE_M_IDS(&intel_ironlake_m_info),
>> > >>  	INTEL_SNB_D_IDS(&intel_sandybridge_d_info),
>> > >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> > >> index 9171431558a3..afe07947e51c 100644
>> > >> --- a/drivers/gpu/drm/i915/intel_pm.c
>> > >> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> > >> @@ -659,7 +659,7 @@ static void pineview_update_wm(struct intel_crtc *unused_crtc)
>> > >>  	u32 reg;
>> > >>  	unsigned long wm;
>> > >>  
>> > >> -	latency = intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
>> > >> +	latency = intel_get_cxsr_latency(!IS_MOBILE(dev_priv),
>> > >>  					 dev_priv->is_ddr3,
>> > >>  					 dev_priv->fsb_freq,
>> > >>  					 dev_priv->mem_freq);
>> > >> @@ -7730,7 +7730,7 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
>> > >>  		vlv_setup_wm_latency(dev_priv);
>> > >>  		dev_priv->display.update_wm = vlv_update_wm;
>> > >>  	} else if (IS_PINEVIEW(dev_priv)) {
>> > >> -		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
>> > >> +		if (!intel_get_cxsr_latency(!IS_MOBILE(dev_priv),
>> > >>  					    dev_priv->is_ddr3,
>> > >>  					    dev_priv->fsb_freq,
>> > >>  					    dev_priv->mem_freq)) {
>> > >> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
>> > >> index 540be9ff0346..9c226eb1788f 100644
>> > >> --- a/include/drm/i915_pciids.h
>> > >> +++ b/include/drm/i915_pciids.h
>> > >> @@ -100,8 +100,10 @@
>> > >>  	INTEL_VGA_DEVICE(0x2e42, info), /* B43_G */ \
>> > >>  	INTEL_VGA_DEVICE(0x2e92, info)	/* B43_G.1 */
>> > >>  
>> > >> -#define INTEL_PINEVIEW_IDS(info)			\
>> > >> -	INTEL_VGA_DEVICE(0xa001, info),			\
>> > >> +#define INTEL_PINEVIEW_D_IDS(info) \
>> > >> +	INTEL_VGA_DEVICE(0xa001, info)
>> > >> +
>> > >> +#define INTEL_PINEVIEW_M_IDS(info) \
>> > >>  	INTEL_VGA_DEVICE(0xa011, info)
>> > >>  
>> > >>  #define INTEL_IRONLAKE_D_IDS(info) \
>> > 
>> > -- 
>> > Jani Nikula, Intel Open Source Technology Center
>> 
>> -- 
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-12-08 21:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-07 22:10 [PATCH 1/2] drm/i915: split off Pineview desktop and mobile device info Jani Nikula
2016-12-07 22:10 ` [PATCH 2/2] drm/i915: reduce PCI ID duplication in IS_IRONLAKE_M() Jani Nikula
2016-12-07 22:19 ` [PATCH 1/2] drm/i915: split off Pineview desktop and mobile device info Jani Nikula
2016-12-08  9:11   ` Jani Nikula
2016-12-08 15:58     ` Daniel Vetter
2016-12-08 16:27       ` Ville Syrjälä
2016-12-08 21:03         ` Jani Nikula
2016-12-07 23:45 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
2016-12-08  6:53   ` Saarinen, Jani

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox