All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>,
	intel-gfx@lists.freedesktop.org
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [PATCH 3/5] drm/i915: merge gen checks to use range
Date: Thu, 1 Nov 2018 11:31:25 +0000	[thread overview]
Message-ID: <4d341fa0-538a-d4aa-29bb-307ce04523ff@linux.intel.com> (raw)
In-Reply-To: <20181101083517.20193-4-lucas.demarchi@intel.com>


On 01/11/2018 08:35, Lucas De Marchi wrote:
> Instead of using several IS_GEN(), it's possible to pass the
> range as argument. By code inspection these were the ranges deemed
> necessary for spatch:
> 
> @@
> expression e;
> @@
> (
> - IS_GEN(e, 3) || IS_GEN(e, 4)
> + IS_GEN(e, 3, 4)
> |
> - IS_GEN(e, 5) || IS_GEN(e, 6)
> + IS_GEN(e, 5, 6)
> |
> - IS_GEN(e, 6) || IS_GEN(e, 7)
> + IS_GEN(e, 6, 7)
> |
> - IS_GEN(e, 7) || IS_GEN(e, 8)
> + IS_GEN(e, 7, 8)
> |
> - IS_GEN(e, 8) || IS_GEN(e, 9)
> + IS_GEN(e, 8, 9)
> |
> - IS_GEN(e, 10) || IS_GEN(e, 9)
> + IS_GEN(e, 9, 10)
> |
> - IS_GEN(e, 9) || IS_GEN(e, 10)
> + IS_GEN(e, 9, 10)
> )
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c        | 6 +++---
>   drivers/gpu/drm/i915/i915_gpu_error.c      | 2 +-
>   drivers/gpu/drm/i915/i915_perf.c           | 2 +-
>   drivers/gpu/drm/i915/intel_crt.c           | 2 +-
>   drivers/gpu/drm/i915/intel_device_info.c   | 2 +-
>   drivers/gpu/drm/i915/intel_display.c       | 2 +-
>   drivers/gpu/drm/i915/intel_engine_cs.c     | 2 +-
>   drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +-
>   drivers/gpu/drm/i915/intel_pipe_crc.c      | 4 ++--
>   drivers/gpu/drm/i915/intel_uncore.c        | 6 +++---
>   10 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 28d95f9d0b0e..f2fbc016bd7f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2030,7 +2030,7 @@ static int i915_swizzle_info(struct seq_file *m, void *data)
>   	seq_printf(m, "bit6 swizzle for Y-tiling = %s\n",
>   		   swizzle_string(dev_priv->mm.bit_6_swizzle_y));
>   
> -	if (IS_GEN(dev_priv, 3) || IS_GEN(dev_priv, 4)) {
> +	if (IS_GEN(dev_priv, 3, 4)) {

I can see value in it but think it would read better with IS_GEN_RANGE.

Are there any cases of or-ed IS_GEN checks with something sandwiched in 
between then, which the above spatch would miss?

How many non-consecutive IS_GEN gen checks are there? To give us some 
ideas if the usual pattern is range, or perhaps checks against a list of 
gens also exists? (Gut feeling says no.)

Regards,

Tvrtko

>   		seq_printf(m, "DDC = 0x%08x\n",
>   			   I915_READ(DCC));
>   		seq_printf(m, "DDC2 = 0x%08x\n",
> @@ -4260,7 +4260,7 @@ i915_cache_sharing_get(void *data, u64 *val)
>   	struct drm_i915_private *dev_priv = data;
>   	u32 snpcr;
>   
> -	if (!(IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7)))
> +	if (!(IS_GEN(dev_priv, 6, 7)))
>   		return -ENODEV;
>   
>   	intel_runtime_pm_get(dev_priv);
> @@ -4280,7 +4280,7 @@ i915_cache_sharing_set(void *data, u64 val)
>   	struct drm_i915_private *dev_priv = data;
>   	u32 snpcr;
>   
> -	if (!(IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7)))
> +	if (!(IS_GEN(dev_priv, 6, 7)))
>   		return -ENODEV;
>   
>   	if (val > 3)
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index c9f8aa47005a..969691e50c04 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1673,7 +1673,7 @@ static void capture_reg_state(struct i915_gpu_state *error)
>   		error->ccid = I915_READ(CCID);
>   
>   	/* 3: Feature specific registers */
> -	if (IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7)) {
> +	if (IS_GEN(dev_priv, 6, 7)) {
>   		error->gam_ecochk = I915_READ(GAM_ECOCHK);
>   		error->gac_eco = I915_READ(GAC_ECO_BITS);
>   	}
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 92daddf79cb0..baaa7b70ffa0 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -3449,7 +3449,7 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
>   		dev_priv->perf.oa.ops.read = gen8_oa_read;
>   		dev_priv->perf.oa.ops.oa_hw_tail_read = gen8_oa_hw_tail_read;
>   
> -		if (IS_GEN(dev_priv, 8) || IS_GEN(dev_priv, 9)) {
> +		if (IS_GEN(dev_priv, 8, 9)) {
>   			dev_priv->perf.oa.ops.is_valid_b_counter_reg =
>   				gen7_is_valid_b_counter_addr;
>   			dev_priv->perf.oa.ops.is_valid_mux_reg =
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index bf4fd739b68c..1822dccb1914 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -322,7 +322,7 @@ intel_crt_mode_valid(struct drm_connector *connector,
>   		 * DAC limit supposedly 355 MHz.
>   		 */
>   		max_clock = 270000;
> -	else if (IS_GEN(dev_priv, 3) || IS_GEN(dev_priv, 4))
> +	else if (IS_GEN(dev_priv, 3, 4))
>   		max_clock = 400000;
>   	else
>   		max_clock = 350000;
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 873f37b7b796..a1b046c322d5 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -783,7 +783,7 @@ void intel_device_info_runtime_init(struct intel_device_info *info)
>   		DRM_INFO("Display disabled (module parameter)\n");
>   		info->num_pipes = 0;
>   	} else if (info->num_pipes > 0 &&
> -		   (IS_GEN(dev_priv, 7) || IS_GEN(dev_priv, 8)) &&
> +		   (IS_GEN(dev_priv, 7, 8)) &&
>   		   HAS_PCH_SPLIT(dev_priv)) {
>   		u32 fuse_strap = I915_READ(FUSE_STRAP);
>   		u32 sfuse_strap = I915_READ(SFUSE_STRAP);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 84c432cbdf5b..02b338b1d8be 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10677,7 +10677,7 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
>   	 * the w/a on all three platforms.
>   	 */
>   	if (plane->id == PLANE_SPRITE0 &&
> -	    (IS_GEN(dev_priv, 5) || IS_GEN(dev_priv, 6) ||
> +	    (IS_GEN(dev_priv, 5, 6) ||
>   	     IS_IVYBRIDGE(dev_priv)) &&
>   	    (turn_on || (!needs_scaling(old_plane_state) &&
>   			 needs_scaling(to_intel_plane_state(plane_state)))))
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 7642f6634f7b..779c683b48ab 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -438,7 +438,7 @@ void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno)
>   	 * the semaphore value, then when the seqno moves backwards all
>   	 * future waits will complete instantly (causing rendering corruption).
>   	 */
> -	if (IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7)) {
> +	if (IS_GEN(dev_priv, 6, 7)) {
>   		I915_WRITE(RING_SYNC_0(engine->mmio_base), 0);
>   		I915_WRITE(RING_SYNC_1(engine->mmio_base), 0);
>   		if (HAS_VEBOX(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> index ff2743ccbece..a16b463a527d 100644
> --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
> +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> @@ -260,7 +260,7 @@ static bool __intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev,
>   
>   	if (HAS_GMCH_DISPLAY(dev_priv))
>   		i9xx_set_fifo_underrun_reporting(dev, pipe, enable, old);
> -	else if (IS_GEN(dev_priv, 5) || IS_GEN(dev_priv, 6))
> +	else if (IS_GEN(dev_priv, 5, 6))
>   		ironlake_set_fifo_underrun_reporting(dev, pipe, enable);
>   	else if (IS_GEN(dev_priv, 7))
>   		ivybridge_set_fifo_underrun_reporting(dev, pipe, enable, old);
> diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
> index 9e870caf8104..3e06570337b6 100644
> --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
> +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
> @@ -433,7 +433,7 @@ static int get_new_crc_ctl_reg(struct drm_i915_private *dev_priv,
>   		return i9xx_pipe_crc_ctl_reg(dev_priv, pipe, source, val);
>   	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>   		return vlv_pipe_crc_ctl_reg(dev_priv, pipe, source, val);
> -	else if (IS_GEN(dev_priv, 5) || IS_GEN(dev_priv, 6))
> +	else if (IS_GEN(dev_priv, 5, 6))
>   		return ilk_pipe_crc_ctl_reg(source, val);
>   	else
>   		return ivb_pipe_crc_ctl_reg(dev_priv, pipe, source, val, set_wa);
> @@ -550,7 +550,7 @@ intel_is_valid_crc_source(struct drm_i915_private *dev_priv,
>   		return i9xx_crc_source_valid(dev_priv, source);
>   	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>   		return vlv_crc_source_valid(dev_priv, source);
> -	else if (IS_GEN(dev_priv, 5) || IS_GEN(dev_priv, 6))
> +	else if (IS_GEN(dev_priv, 5, 6))
>   		return ilk_crc_source_valid(dev_priv, source);
>   	else
>   		return ivb_crc_source_valid(dev_priv, source);
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index c36453d66d93..88cbd32d6964 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -528,7 +528,7 @@ check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
>   	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>   		ret |= vlv_check_for_unclaimed_mmio(dev_priv);
>   
> -	if (IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7))
> +	if (IS_GEN(dev_priv, 6, 7))
>   		ret |= gen6_check_for_fifo_debug(dev_priv);
>   
>   	return ret;
> @@ -556,7 +556,7 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
>   		dev_priv->uncore.funcs.force_wake_get(dev_priv,
>   						      restore_forcewake);
>   
> -		if (IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7))
> +		if (IS_GEN(dev_priv, 6, 7))
>   			dev_priv->uncore.fifo_count =
>   				fifo_free_entries(dev_priv);
>   		spin_unlock_irq(&dev_priv->uncore.lock);
> @@ -1437,7 +1437,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
>   				       FORCEWAKE_MEDIA_VEBOX_GEN11(i),
>   				       FORCEWAKE_ACK_MEDIA_VEBOX_GEN11(i));
>   		}
> -	} else if (IS_GEN(dev_priv, 10) || IS_GEN(dev_priv, 9)) {
> +	} else if (IS_GEN(dev_priv, 9, 10)) {
>   		dev_priv->uncore.funcs.force_wake_get =
>   			fw_domains_get_with_fallback;
>   		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2018-11-01 11:31 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-01  8:35 [PATCH 0/5] Make GEN macros more similar Lucas De Marchi
2018-11-01  8:35 ` [PATCH 1/5] drm/i915: let IS_GEN receive 1 or 2 arguments Lucas De Marchi
2018-11-01  8:35 ` [PATCH 2/5] drm/i915: replace IS_GEN<N> with IS_GEN(..., N) Lucas De Marchi
2018-11-01 11:17   ` Jani Nikula
2018-11-01 13:27     ` Ville Syrjälä
2018-11-01 11:35   ` Tvrtko Ursulin
2018-11-01 17:00     ` Lucas De Marchi
2018-11-01  8:35 ` [PATCH 3/5] drm/i915: merge gen checks to use range Lucas De Marchi
2018-11-01 11:18   ` Jani Nikula
2018-11-01 11:31   ` Tvrtko Ursulin [this message]
2018-11-02 18:10     ` Lucas De Marchi
2018-11-02 19:19       ` Rodrigo Vivi
2018-11-02 19:47         ` Lucas De Marchi
2018-11-02 22:12           ` Rodrigo Vivi
2018-11-02 22:28             ` Lucas De Marchi
2018-11-02 22:47               ` Rodrigo Vivi
2018-11-01  8:35 ` [PATCH 4/5] drm/i915: add helper IS_GEN_* macros Lucas De Marchi
2018-11-01 11:04   ` Tvrtko Ursulin
2018-11-01 11:29     ` Jani Nikula
2018-11-01  8:35 ` [PATCH 5/5] RFC: drm/i915: convert a few cases to the " Lucas De Marchi
2018-11-01  9:30 ` ✗ Fi.CI.CHECKPATCH: warning for Make GEN macros more similar Patchwork
2018-11-01  9:32 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-11-01  9:57 ` ✗ Fi.CI.BAT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4d341fa0-538a-d4aa-29bb-307ce04523ff@linux.intel.com \
    --to=tvrtko.ursulin@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lucas.demarchi@intel.com \
    --cc=rodrigo.vivi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.