diff mbox

[05/26] drm/i915: extract fbc_on_pipe_a_only()

Message ID 1445964628-30226-6-git-send-email-paulo.r.zanoni@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zanoni, Paulo R Oct. 27, 2015, 4:50 p.m. UTC
Make the code easier to read.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_fbc.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Maarten Lankhorst Oct. 29, 2015, 12:05 p.m. UTC | #1
Op 27-10-15 om 17:50 schreef Paulo Zanoni:
> Make the code easier to read.
>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 7d8e996..4d6ebc7 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -46,6 +46,11 @@ static inline bool fbc_supported(struct drm_i915_private *dev_priv)
>  	return dev_priv->fbc.enable_fbc != NULL;
>  }
>  
> +static inline bool fbc_on_pipe_a_only(struct drm_i915_private *dev_priv)
> +{
> +	return IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8;
> +}
This check is duplicated in __intel_fbc_update, which also seems to say < gen4 doesn't support it but that's not mentioned here.

I would say use possible_framebuffer_bits and remove this extra check here in the future.

I'll probably rework this later though, so this is fine with me for now.
>  /*
>   * In some platforms where the CRTC's x:0/y:0 coordinates doesn't match the
>   * frontbuffer's x:0/y:0 coordinates we lie to the hardware about the plane's
> @@ -543,10 +548,6 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_crtc *crtc = NULL, *tmp_crtc;
>  	enum pipe pipe;
> -	bool pipe_a_only = false;
> -
> -	if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
> -		pipe_a_only = true;
>  
>  	for_each_pipe(dev_priv, pipe) {
>  		tmp_crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> @@ -555,7 +556,7 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
>  		    to_intel_plane_state(tmp_crtc->primary->state)->visible)
>  			crtc = tmp_crtc;
>  
> -		if (pipe_a_only)
> +		if (fbc_on_pipe_a_only(dev_priv))
>  			break;
>  	}
>  
> @@ -1146,7 +1147,7 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
>  		dev_priv->fbc.possible_framebuffer_bits |=
>  				INTEL_FRONTBUFFER_PRIMARY(pipe);
>  
> -		if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
> +		if (fbc_on_pipe_a_only(dev_priv))
>  			break;
>  	}
>
Zanoni, Paulo R Oct. 29, 2015, 3:55 p.m. UTC | #2
Em Qui, 2015-10-29 às 13:05 +0100, Maarten Lankhorst escreveu:
> Op 27-10-15 om 17:50 schreef Paulo Zanoni:

> > Make the code easier to read.

> > 

> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>

> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> > ---

> >  drivers/gpu/drm/i915/intel_fbc.c | 13 +++++++------

> >  1 file changed, 7 insertions(+), 6 deletions(-)

> > 

> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c

> > b/drivers/gpu/drm/i915/intel_fbc.c

> > index 7d8e996..4d6ebc7 100644

> > --- a/drivers/gpu/drm/i915/intel_fbc.c

> > +++ b/drivers/gpu/drm/i915/intel_fbc.c

> > @@ -46,6 +46,11 @@ static inline bool fbc_supported(struct

> > drm_i915_private *dev_priv)

> >  	return dev_priv->fbc.enable_fbc != NULL;

> >  }

> >  

> > +static inline bool fbc_on_pipe_a_only(struct drm_i915_private

> > *dev_priv)

> > +{

> > +	return IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen

> > >= 8;

> > +}

> This check is duplicated in __intel_fbc_update, which also seems to

> say < gen4 doesn't support it but that's not mentioned here.

> 

> I would say use possible_framebuffer_bits and remove this extra check

> here in the future.



Pipes != planes on gen < 4. I think your suggestion is related to patch
21/26

> 

> I'll probably rework this later though, so this is fine with me for

> now.

> >  /*

> >   * In some platforms where the CRTC's x:0/y:0 coordinates doesn't

> > match the

> >   * frontbuffer's x:0/y:0 coordinates we lie to the hardware about

> > the plane's

> > @@ -543,10 +548,6 @@ static struct drm_crtc

> > *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)

> >  {

> >  	struct drm_crtc *crtc = NULL, *tmp_crtc;

> >  	enum pipe pipe;

> > -	bool pipe_a_only = false;

> > -

> > -	if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >=

> > 8)

> > -		pipe_a_only = true;

> >  

> >  	for_each_pipe(dev_priv, pipe) {

> >  		tmp_crtc = dev_priv->pipe_to_crtc_mapping[pipe];

> > @@ -555,7 +556,7 @@ static struct drm_crtc

> > *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)

> >  		    to_intel_plane_state(tmp_crtc->primary-

> > >state)->visible)

> >  			crtc = tmp_crtc;

> >  

> > -		if (pipe_a_only)

> > +		if (fbc_on_pipe_a_only(dev_priv))

> >  			break;

> >  	}

> >  

> > @@ -1146,7 +1147,7 @@ void intel_fbc_init(struct drm_i915_private

> > *dev_priv)

> >  		dev_priv->fbc.possible_framebuffer_bits |=

> >  				INTEL_FRONTBUFFER_PRIMARY(pipe);

> >  

> > -		if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)-

> > >gen >= 8)

> > +		if (fbc_on_pipe_a_only(dev_priv))

> >  			break;

> >  	}

> >  

>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 7d8e996..4d6ebc7 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -46,6 +46,11 @@  static inline bool fbc_supported(struct drm_i915_private *dev_priv)
 	return dev_priv->fbc.enable_fbc != NULL;
 }
 
+static inline bool fbc_on_pipe_a_only(struct drm_i915_private *dev_priv)
+{
+	return IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8;
+}
+
 /*
  * In some platforms where the CRTC's x:0/y:0 coordinates doesn't match the
  * frontbuffer's x:0/y:0 coordinates we lie to the hardware about the plane's
@@ -543,10 +548,6 @@  static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
 {
 	struct drm_crtc *crtc = NULL, *tmp_crtc;
 	enum pipe pipe;
-	bool pipe_a_only = false;
-
-	if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
-		pipe_a_only = true;
 
 	for_each_pipe(dev_priv, pipe) {
 		tmp_crtc = dev_priv->pipe_to_crtc_mapping[pipe];
@@ -555,7 +556,7 @@  static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
 		    to_intel_plane_state(tmp_crtc->primary->state)->visible)
 			crtc = tmp_crtc;
 
-		if (pipe_a_only)
+		if (fbc_on_pipe_a_only(dev_priv))
 			break;
 	}
 
@@ -1146,7 +1147,7 @@  void intel_fbc_init(struct drm_i915_private *dev_priv)
 		dev_priv->fbc.possible_framebuffer_bits |=
 				INTEL_FRONTBUFFER_PRIMARY(pipe);
 
-		if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
+		if (fbc_on_pipe_a_only(dev_priv))
 			break;
 	}