[Intel-gfx] [PATCH 2/3] drm/i915: get rid of dev_priv-info-has_pch_split
From: Paulo Zanoni paulo.r.zan...@intel.com
Previously we had has_pch_split to tell us whether we had a PCH or not
and we also had dev_priv-pch_type to tell us which kind of PCH it
was, but it could only be used if we were 100% sure we did have a PCH.
Now that PCH_NONE was added to dev_priv-pch_type we don't need
has_pch_split anymore: we can just check for pch_type != PCH_NONE.
The HAS_PCH_{IBX,CPT,LPT} macros use dev_priv-pch_type, so they can
only be called after intel_detect_pch. The HAS_PCH_SPLIT macro looks
at dev_priv-info-has_pch_split, which is available earlier.
Since the goal is to implement HAS_PCH_SPLIT using dev_priv-pch_type
instead of dev_priv-info-has_pch_split, we need to make sure that
intel_detect_pch is called before any calls to HAS_PCH_SPLIT are made.
So we moved the intel_detect_pch call to an earlier stage.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_dma.c |5 +++--
drivers/gpu/drm/i915/i915_drv.c |8
drivers/gpu/drm/i915/i915_drv.h |3 +--
3 files changed, 4 insertions(+), 12 deletions(-)
This patch does not solve any real problem: it's just a suggestion
of something we could do after the previous patch. Some people may
argue that looking at the has_pch_split variable might make it easier
for us to find out which machines actually have a pch split without
running the machine. So I really won't complain if we don't accept
this patch: patch 01 is the important one.
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2166519..f8bc9ea 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1547,6 +1547,9 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
goto out_mtrrfree;
}
+ /* This must be called before any calls to HAS_PCH_* */
+ intel_detect_pch(dev);
+
intel_irq_init(dev);
/* Try to make sure MCHBAR is enabled before poking at it */
@@ -1599,8 +1602,6 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
/* Start out suspended */
dev_priv-mm.suspended = 1;
- intel_detect_pch(dev);
-
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
ret = i915_load_modeset_init(dev);
if (ret 0) {
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7d0eb82..1794833 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -225,7 +225,6 @@ static const struct intel_device_info intel_ironlake_d_info
= {
.gen = 5,
.need_gfx_hws = 1, .has_hotplug = 1,
.has_bsd_ring = 1,
- .has_pch_split = 1,
};
static const struct intel_device_info intel_ironlake_m_info = {
@@ -233,7 +232,6 @@ static const struct intel_device_info intel_ironlake_m_info
= {
.need_gfx_hws = 1, .has_hotplug = 1,
.has_fbc = 1,
.has_bsd_ring = 1,
- .has_pch_split = 1,
};
static const struct intel_device_info intel_sandybridge_d_info = {
@@ -242,7 +240,6 @@ static const struct intel_device_info
intel_sandybridge_d_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -253,7 +250,6 @@ static const struct intel_device_info
intel_sandybridge_m_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -263,7 +259,6 @@ static const struct intel_device_info
intel_ivybridge_d_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -274,7 +269,6 @@ static const struct intel_device_info
intel_ivybridge_m_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -302,7 +296,6 @@ static const struct intel_device_info intel_haswell_d_info
= {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -312,7 +305,6 @@ static const struct intel_device_info intel_haswell_m_info
= {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b12e79a..89025ab 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -285,7 +285,6 @@ struct intel_device_info {
u8 is_crestline:1;
u8 is_ivybridge:1;
u8 is_valleyview:1;
- u8 has_pch_split:1;
u8 has_force_wake:1;
u8 is_haswell:1;
u8 has_fbc:1;
@@ -1113,13 +1112,13 @@ struct drm_i915_file_private {
#define HAS_PIPE_CXSR(dev) (INTEL_INFO(dev)-has_pipe_cxsr)
#define I915_HAS_FBC(dev)
Re: [Intel-gfx] [PATCH 2/3] drm/i915: get rid of dev_priv-info-has_pch_split
On Tue, Jul 03, 2012 at 03:57:32PM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
Previously we had has_pch_split to tell us whether we had a PCH or not
and we also had dev_priv-pch_type to tell us which kind of PCH it
was, but it could only be used if we were 100% sure we did have a PCH.
Now that PCH_NONE was added to dev_priv-pch_type we don't need
has_pch_split anymore: we can just check for pch_type != PCH_NONE.
The HAS_PCH_{IBX,CPT,LPT} macros use dev_priv-pch_type, so they can
only be called after intel_detect_pch. The HAS_PCH_SPLIT macro looks
at dev_priv-info-has_pch_split, which is available earlier.
Since the goal is to implement HAS_PCH_SPLIT using dev_priv-pch_type
instead of dev_priv-info-has_pch_split, we need to make sure that
intel_detect_pch is called before any calls to HAS_PCH_SPLIT are made.
So we moved the intel_detect_pch call to an earlier stage.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_dma.c |5 +++--
drivers/gpu/drm/i915/i915_drv.c |8
drivers/gpu/drm/i915/i915_drv.h |3 +--
3 files changed, 4 insertions(+), 12 deletions(-)
This patch does not solve any real problem: it's just a suggestion
of something we could do after the previous patch. Some people may
argue that looking at the has_pch_split variable might make it easier
for us to find out which machines actually have a pch split without
running the machine. So I really won't complain if we don't accept
this patch: patch 01 is the important one.
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2166519..f8bc9ea 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1547,6 +1547,9 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
goto out_mtrrfree;
}
+ /* This must be called before any calls to HAS_PCH_* */
+ intel_detect_pch(dev);
+
Hm, what about defining PCH_NONE as -1, PCH_RESERVED as 0 and then adding
a WARN_ON(dev_priv-pch_type == PCH_RESERVED)? detect_pch is called
unconditionally, and that way we would catch this. Might be overkill otoh,
so if you think this is not worth it, np.
-Daniel
intel_irq_init(dev);
/* Try to make sure MCHBAR is enabled before poking at it */
@@ -1599,8 +1602,6 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
/* Start out suspended */
dev_priv-mm.suspended = 1;
- intel_detect_pch(dev);
-
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
ret = i915_load_modeset_init(dev);
if (ret 0) {
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7d0eb82..1794833 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -225,7 +225,6 @@ static const struct intel_device_info
intel_ironlake_d_info = {
.gen = 5,
.need_gfx_hws = 1, .has_hotplug = 1,
.has_bsd_ring = 1,
- .has_pch_split = 1,
};
static const struct intel_device_info intel_ironlake_m_info = {
@@ -233,7 +232,6 @@ static const struct intel_device_info
intel_ironlake_m_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.has_fbc = 1,
.has_bsd_ring = 1,
- .has_pch_split = 1,
};
static const struct intel_device_info intel_sandybridge_d_info = {
@@ -242,7 +240,6 @@ static const struct intel_device_info
intel_sandybridge_d_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -253,7 +250,6 @@ static const struct intel_device_info
intel_sandybridge_m_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -263,7 +259,6 @@ static const struct intel_device_info
intel_ivybridge_d_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -274,7 +269,6 @@ static const struct intel_device_info
intel_ivybridge_m_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -302,7 +296,6 @@ static const struct intel_device_info
intel_haswell_d_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -312,7 +305,6 @@ static const struct intel_device_info
intel_haswell_m_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b12e79a..89025ab 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -285,7 +285,6 @@ struct
Re: [Intel-gfx] [PATCH 2/3] drm/i915: get rid of dev_priv-info-has_pch_split
2012/7/3 Daniel Vetter dan...@ffwll.ch:
Hm, what about defining PCH_NONE as -1, PCH_RESERVED as 0 and then adding
a WARN_ON(dev_priv-pch_type == PCH_RESERVED)? detect_pch is called
unconditionally, and that way we would catch this. Might be overkill otoh,
so if you think this is not worth it, np.
I actually thought about this idea before. But it would only make
sense if we add these WARNs to the HAS_PCH_FOO macros. But these
macros are supposed to be simple and cheap and fast... Do we really
want to start adding assertions inside them? If we think the price is
worth paying, then we might do as you suggested. Or maybe this could
be inside some #ifdef DEBUG...
On my local machines, I changed the HAS_PCH_FOO macros to print some
stuff so I could check whether any of them was called before
intel_detect_pch. At least on these machines (SNB, HSW and a netbook),
everything was fine.
-Daniel
intel_irq_init(dev);
/* Try to make sure MCHBAR is enabled before poking at it */
@@ -1599,8 +1602,6 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
/* Start out suspended */
dev_priv-mm.suspended = 1;
- intel_detect_pch(dev);
-
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
ret = i915_load_modeset_init(dev);
if (ret 0) {
diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index 7d0eb82..1794833 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -225,7 +225,6 @@ static const struct intel_device_info
intel_ironlake_d_info = {
.gen = 5,
.need_gfx_hws = 1, .has_hotplug = 1,
.has_bsd_ring = 1,
- .has_pch_split = 1,
};
static const struct intel_device_info intel_ironlake_m_info = {
@@ -233,7 +232,6 @@ static const struct intel_device_info
intel_ironlake_m_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.has_fbc = 1,
.has_bsd_ring = 1,
- .has_pch_split = 1,
};
static const struct intel_device_info intel_sandybridge_d_info = {
@@ -242,7 +240,6 @@ static const struct intel_device_info
intel_sandybridge_d_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -253,7 +250,6 @@ static const struct intel_device_info
intel_sandybridge_m_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -263,7 +259,6 @@ static const struct intel_device_info
intel_ivybridge_d_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -274,7 +269,6 @@ static const struct intel_device_info
intel_ivybridge_m_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -302,7 +296,6 @@ static const struct intel_device_info
intel_haswell_d_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
@@ -312,7 +305,6 @@ static const struct intel_device_info
intel_haswell_m_info = {
.has_bsd_ring = 1,
.has_blt_ring = 1,
.has_llc = 1,
- .has_pch_split = 1,
.has_force_wake = 1,
};
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index b12e79a..89025ab 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -285,7 +285,6 @@ struct intel_device_info {
u8 is_crestline:1;
u8 is_ivybridge:1;
u8 is_valleyview:1;
- u8 has_pch_split:1;
u8 has_force_wake:1;
u8 is_haswell:1;
u8 has_fbc:1;
@@ -1113,13 +1112,13 @@ struct drm_i915_file_private {
#define HAS_PIPE_CXSR(dev) (INTEL_INFO(dev)-has_pipe_cxsr)
#define I915_HAS_FBC(dev) (INTEL_INFO(dev)-has_fbc)
-#define HAS_PCH_SPLIT(dev) (INTEL_INFO(dev)-has_pch_split)
#define HAS_PIPE_CONTROL(dev) (INTEL_INFO(dev)-gen = 5)
#define INTEL_PCH_TYPE(dev) (((struct drm_i915_private
*)(dev)-dev_private)-pch_type)
#define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT)
#define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT)
#define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX)
+#define HAS_PCH_SPLIT(dev) (INTEL_PCH_TYPE(dev) != PCH_NONE)
#define HAS_FORCE_WAKE(dev) (INTEL_INFO(dev)-has_force_wake)
--
1.7.10.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
