diff mbox

[08/16] drm/i915: Ivy Bridge has split display and pipe control

Message ID 1303861134-8762-9-git-send-email-jbarnes@virtuousgeek.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jesse Barnes April 26, 2011, 11:38 p.m. UTC
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(-)

Comments

Chris Wilson April 27, 2011, 7:19 a.m. UTC | #1
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
Daniel Vetter April 27, 2011, 8:02 p.m. UTC | #2
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
Jesse Barnes April 27, 2011, 9:03 p.m. UTC | #3
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 mbox

Patch

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)