| Message ID | 1303861134-8762-9-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 26 Apr 2011 16:38:46 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > Ivy Bridge has a similar split display controller to Sandy Bridge, so > use HAS_PCH_SPLIT. And gen7 also has the pipe control instruction, so > use HAS_PIPE_CONTROL as well. > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/i915_drv.h | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 9fbb6fe..e596c10 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -965,8 +965,8 @@ enum intel_chip_family { > #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) (IS_GEN5(dev) || IS_GEN6(dev)) > -#define HAS_PIPE_CONTROL(dev) (IS_GEN5(dev) || IS_GEN6(dev)) > +#define HAS_PCH_SPLIT(dev) (IS_GEN5(dev) || IS_GEN6(dev) || IS_IVYBRIDGE(dev)) > +#define HAS_PIPE_CONTROL(dev) (IS_GEN5(dev) || IS_GEN6(dev) || IS_GEN7(dev)) > > #define INTEL_PCH_TYPE(dev) (((struct drm_i915_private *)(dev)->dev_private)->pch_type) > #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT) So either we are confident that every future ILK+ continues with the split and keeps pipe_control, in which case we do the obvious simplification or we make these an actual capability bit before the code becomes a deep nesting of predicates again... -Chris
On Wed, Apr 27, 2011 at 08:19:21AM +0100, Chris Wilson wrote: > On Tue, 26 Apr 2011 16:38:46 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > > Ivy Bridge has a similar split display controller to Sandy Bridge, so > > use HAS_PCH_SPLIT. And gen7 also has the pipe control instruction, so > > use HAS_PIPE_CONTROL as well. > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 4 ++-- > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 9fbb6fe..e596c10 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -965,8 +965,8 @@ enum intel_chip_family { > > #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) (IS_GEN5(dev) || IS_GEN6(dev)) > > -#define HAS_PIPE_CONTROL(dev) (IS_GEN5(dev) || IS_GEN6(dev)) > > +#define HAS_PCH_SPLIT(dev) (IS_GEN5(dev) || IS_GEN6(dev) || IS_IVYBRIDGE(dev)) > > +#define HAS_PIPE_CONTROL(dev) (IS_GEN5(dev) || IS_GEN6(dev) || IS_GEN7(dev)) > > > > #define INTEL_PCH_TYPE(dev) (((struct drm_i915_private *)(dev)->dev_private)->pch_type) > > #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT) > > So either we are confident that every future ILK+ continues with the split > and keeps pipe_control, in which case we do the obvious simplification or > we make these an actual capability bit before the code becomes a deep > nesting of predicates again... HAS_PIPE_CONTROL is currently unused in the kernel. We might as well kill it. Unreleated style critique that also applies to other patches: I think checks of the from (gen >= 5) would be easier to read - using IS_FOO macros always looks a bit like a special case ... -Daniel
On Wed, 27 Apr 2011 08:19:21 +0100 Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Tue, 26 Apr 2011 16:38:46 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > > Ivy Bridge has a similar split display controller to Sandy Bridge, so > > use HAS_PCH_SPLIT. And gen7 also has the pipe control instruction, so > > use HAS_PIPE_CONTROL as well. > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 4 ++-- > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 9fbb6fe..e596c10 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -965,8 +965,8 @@ enum intel_chip_family { > > #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) (IS_GEN5(dev) || IS_GEN6(dev)) > > -#define HAS_PIPE_CONTROL(dev) (IS_GEN5(dev) || IS_GEN6(dev)) > > +#define HAS_PCH_SPLIT(dev) (IS_GEN5(dev) || IS_GEN6(dev) || IS_IVYBRIDGE(dev)) > > +#define HAS_PIPE_CONTROL(dev) (IS_GEN5(dev) || IS_GEN6(dev) || IS_GEN7(dev)) > > > > #define INTEL_PCH_TYPE(dev) (((struct drm_i915_private *)(dev)->dev_private)->pch_type) > > #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT) > > So either we are confident that every future ILK+ continues with the split > and keeps pipe_control, in which case we do the obvious simplification or > we make these an actual capability bit before the code becomes a deep > nesting of predicates again... No, I expect HAS_PCH_SPLIT won't apply to upcoming gen7 chipsets. The cantiga display controller is even used in GMA500, so it really is separate from render related checks. We could probably come up with better names though to make things more readable and consistent. Jesse
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9fbb6fe..e596c10 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -965,8 +965,8 @@ enum intel_chip_family { #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) (IS_GEN5(dev) || IS_GEN6(dev)) -#define HAS_PIPE_CONTROL(dev) (IS_GEN5(dev) || IS_GEN6(dev)) +#define HAS_PCH_SPLIT(dev) (IS_GEN5(dev) || IS_GEN6(dev) || IS_IVYBRIDGE(dev)) +#define HAS_PIPE_CONTROL(dev) (IS_GEN5(dev) || IS_GEN6(dev) || IS_GEN7(dev)) #define INTEL_PCH_TYPE(dev) (((struct drm_i915_private *)(dev)->dev_private)->pch_type) #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT)
Ivy Bridge has a similar split display controller to Sandy Bridge, so use HAS_PCH_SPLIT. And gen7 also has the pipe control instruction, so use HAS_PIPE_CONTROL as well. Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/i915_drv.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)