[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