diff mbox

drm/i915: Remove superfluous hw_flags from mi_set_context()

Message ID 20170322210350.6208-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson March 22, 2017, 9:03 p.m. UTC
Why have both hw_flags and flags, when just one will do?

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Tvrtko Ursulin March 23, 2017, 9:37 a.m. UTC | #1
On 22/03/2017 21:03, Chris Wilson wrote:
> Why have both hw_flags and flags, when just one will do?
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 486051ed681d..8fc8b3d15a0f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -576,25 +576,25 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
>  }
>
>  static inline int
> -mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
> +mi_set_context(struct drm_i915_gem_request *req, u32 flags)
>  {
>  	struct drm_i915_private *dev_priv = req->i915;
>  	struct intel_engine_cs *engine = req->engine;
>  	enum intel_engine_id id;
> -	u32 *cs, flags = hw_flags | MI_MM_SPACE_GTT;
>  	const int num_rings =
>  		/* Use an extended w/a on ivb+ if signalling from other rings */
>  		i915.semaphores ?
>  		INTEL_INFO(dev_priv)->num_rings - 1 :
>  		0;
>  	int len;
> +	u32 *cs;
>
> -	/* These flags are for resource streamer on HSW+ */
> +	flags |= MI_MM_SPACE_GTT;
>  	if (IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8)
> -		flags |= (HSW_MI_RS_SAVE_STATE_EN | HSW_MI_RS_RESTORE_STATE_EN);
> -	else if (INTEL_GEN(dev_priv) < 8)
> -		flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);
> -
> +		/* These flags are for resource streamer on HSW+ */
> +		flags |= HSW_MI_RS_SAVE_STATE_EN | HSW_MI_RS_RESTORE_STATE_EN;
> +	else
> +		flags |= MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN;
>
>  	len = 4;
>  	if (INTEL_GEN(dev_priv) >= 7)
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
Chris Wilson March 23, 2017, 10:03 a.m. UTC | #2
On Thu, Mar 23, 2017 at 09:37:35AM +0000, Tvrtko Ursulin wrote:
> 
> On 22/03/2017 21:03, Chris Wilson wrote:
> >Why have both hw_flags and flags, when just one will do?
> >
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >---
> > drivers/gpu/drm/i915/i915_gem_context.c | 14 +++++++-------
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> >index 486051ed681d..8fc8b3d15a0f 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_context.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_context.c
> >@@ -576,25 +576,25 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
> > }
> >
> > static inline int
> >-mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
> >+mi_set_context(struct drm_i915_gem_request *req, u32 flags)
> > {
> > 	struct drm_i915_private *dev_priv = req->i915;
> > 	struct intel_engine_cs *engine = req->engine;
> > 	enum intel_engine_id id;
> >-	u32 *cs, flags = hw_flags | MI_MM_SPACE_GTT;
> > 	const int num_rings =
> > 		/* Use an extended w/a on ivb+ if signalling from other rings */
> > 		i915.semaphores ?
> > 		INTEL_INFO(dev_priv)->num_rings - 1 :
> > 		0;
> > 	int len;
> >+	u32 *cs;
> >
> >-	/* These flags are for resource streamer on HSW+ */
> >+	flags |= MI_MM_SPACE_GTT;
> > 	if (IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8)
> >-		flags |= (HSW_MI_RS_SAVE_STATE_EN | HSW_MI_RS_RESTORE_STATE_EN);
> >-	else if (INTEL_GEN(dev_priv) < 8)
> >-		flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);
> >-
> >+		/* These flags are for resource streamer on HSW+ */
> >+		flags |= HSW_MI_RS_SAVE_STATE_EN | HSW_MI_RS_RESTORE_STATE_EN;
> >+	else
> >+		flags |= MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN;
> >
> > 	len = 4;
> > 	if (INTEL_GEN(dev_priv) >= 7)
> >
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Thanks, those brackets had irked me for a long time :)
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 486051ed681d..8fc8b3d15a0f 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -576,25 +576,25 @@  void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
 }
 
 static inline int
-mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
+mi_set_context(struct drm_i915_gem_request *req, u32 flags)
 {
 	struct drm_i915_private *dev_priv = req->i915;
 	struct intel_engine_cs *engine = req->engine;
 	enum intel_engine_id id;
-	u32 *cs, flags = hw_flags | MI_MM_SPACE_GTT;
 	const int num_rings =
 		/* Use an extended w/a on ivb+ if signalling from other rings */
 		i915.semaphores ?
 		INTEL_INFO(dev_priv)->num_rings - 1 :
 		0;
 	int len;
+	u32 *cs;
 
-	/* These flags are for resource streamer on HSW+ */
+	flags |= MI_MM_SPACE_GTT;
 	if (IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8)
-		flags |= (HSW_MI_RS_SAVE_STATE_EN | HSW_MI_RS_RESTORE_STATE_EN);
-	else if (INTEL_GEN(dev_priv) < 8)
-		flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);
-
+		/* These flags are for resource streamer on HSW+ */
+		flags |= HSW_MI_RS_SAVE_STATE_EN | HSW_MI_RS_RESTORE_STATE_EN;
+	else
+		flags |= MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN;
 
 	len = 4;
 	if (INTEL_GEN(dev_priv) >= 7)