All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org
Subject: Re: [Intel-gfx] [PATCH 3/6] drm/i915: Widen the QGV point mask
Date: Mon, 14 Feb 2022 19:17:32 +0200	[thread overview]
Message-ID: <20220214171732.GA25816@intel.com> (raw)
In-Reply-To: <20220214091811.13725-4-ville.syrjala@linux.intel.com>

On Mon, Feb 14, 2022 at 11:18:08AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> adlp+ adds some extra bits to the QGV point mask. The code attempts
> to handle that but forgot to actually make sure we can store those
> bits in the bw state. Fix it.
> 
> Cc: stable@vger.kernel.org
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Fixes: 192fbfb76744 ("drm/i915: Implement PSF GV point support")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
> index 46c6eecbd917..0ceaed1c9656 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.h
> +++ b/drivers/gpu/drm/i915/display/intel_bw.h
> @@ -30,19 +30,19 @@ struct intel_bw_state {
>  	 */
>  	u8 pipe_sagv_reject;
>  
> +	/* bitmask of active pipes */
> +	u8 active_pipes;
> +
>  	/*
>  	 * Current QGV points mask, which restricts
>  	 * some particular SAGV states, not to confuse
>  	 * with pipe_sagv_mask.
>  	 */
> -	u8 qgv_points_mask;
> +	u16 qgv_points_mask;

Weird, that this went unnoticed. Don't we have static analyzer for such
purposes? Wonder if it should catch and warn about this, because in
intel_bw_atomic_check we have u32 bitmask, which is then getting packed
in 8 bit field.
Probably bitmask type width used in intel_bw_atomic_check should match
that one, so that there would be less room for confusion.

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

>  
>  	unsigned int data_rate[I915_MAX_PIPES];
>  	u8 num_active_planes[I915_MAX_PIPES];
>  
> -	/* bitmask of active pipes */
> -	u8 active_pipes;
> -
>  	int min_cdclk;
>  };
>  
> -- 
> 2.34.1
> 

WARNING: multiple messages have this Message-ID (diff)
From: "Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org
Subject: Re: [PATCH 3/6] drm/i915: Widen the QGV point mask
Date: Mon, 14 Feb 2022 19:17:32 +0200	[thread overview]
Message-ID: <20220214171732.GA25816@intel.com> (raw)
In-Reply-To: <20220214091811.13725-4-ville.syrjala@linux.intel.com>

On Mon, Feb 14, 2022 at 11:18:08AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> adlp+ adds some extra bits to the QGV point mask. The code attempts
> to handle that but forgot to actually make sure we can store those
> bits in the bw state. Fix it.
> 
> Cc: stable@vger.kernel.org
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Fixes: 192fbfb76744 ("drm/i915: Implement PSF GV point support")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
> index 46c6eecbd917..0ceaed1c9656 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.h
> +++ b/drivers/gpu/drm/i915/display/intel_bw.h
> @@ -30,19 +30,19 @@ struct intel_bw_state {
>  	 */
>  	u8 pipe_sagv_reject;
>  
> +	/* bitmask of active pipes */
> +	u8 active_pipes;
> +
>  	/*
>  	 * Current QGV points mask, which restricts
>  	 * some particular SAGV states, not to confuse
>  	 * with pipe_sagv_mask.
>  	 */
> -	u8 qgv_points_mask;
> +	u16 qgv_points_mask;

Weird, that this went unnoticed. Don't we have static analyzer for such
purposes? Wonder if it should catch and warn about this, because in
intel_bw_atomic_check we have u32 bitmask, which is then getting packed
in 8 bit field.
Probably bitmask type width used in intel_bw_atomic_check should match
that one, so that there would be less room for confusion.

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

>  
>  	unsigned int data_rate[I915_MAX_PIPES];
>  	u8 num_active_planes[I915_MAX_PIPES];
>  
> -	/* bitmask of active pipes */
> -	u8 active_pipes;
> -
>  	int min_cdclk;
>  };
>  
> -- 
> 2.34.1
> 

  reply	other threads:[~2022-02-14 17:17 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-14  9:18 [Intel-gfx] [PATCH 0/6] drm/i915: SAGV fixes Ville Syrjala
2022-02-14  9:18 ` [Intel-gfx] [PATCH 1/6] drm/i915: Correctly populate use_sagv_wm for all pipes Ville Syrjala
2022-02-14  9:18   ` Ville Syrjala
2022-02-14 10:16   ` [Intel-gfx] " Lisovskiy, Stanislav
2022-02-14 10:16     ` Lisovskiy, Stanislav
2022-02-14  9:18 ` [Intel-gfx] [PATCH 2/6] drm/i915: Fix bw atomic check when switching between SAGV vs. no SAGV Ville Syrjala
2022-02-14  9:18   ` Ville Syrjala
2022-02-14 10:05   ` [Intel-gfx] " Lisovskiy, Stanislav
2022-02-14 10:05     ` Lisovskiy, Stanislav
2022-02-14 10:24     ` [Intel-gfx] " Ville Syrjälä
2022-02-14 10:24       ` Ville Syrjälä
2022-02-14 17:03       ` [Intel-gfx] " Lisovskiy, Stanislav
2022-02-14 17:03         ` Lisovskiy, Stanislav
2022-02-14 20:26         ` [Intel-gfx] " Ville Syrjälä
2022-02-14 20:26           ` Ville Syrjälä
2022-02-15  8:59           ` [Intel-gfx] " Lisovskiy, Stanislav
2022-02-15  8:59             ` Lisovskiy, Stanislav
2022-02-15 10:10             ` [Intel-gfx] " Ville Syrjälä
2022-02-15 10:10               ` Ville Syrjälä
2022-02-15 11:02               ` [Intel-gfx] " Lisovskiy, Stanislav
2022-02-15 11:02                 ` Lisovskiy, Stanislav
2022-02-15 11:26                 ` [Intel-gfx] " Ville Syrjälä
2022-02-15 11:26                   ` Ville Syrjälä
2022-02-15 16:33                   ` [Intel-gfx] " Lisovskiy, Stanislav
2022-02-15 16:33                     ` Lisovskiy, Stanislav
2022-02-15 16:52                     ` [Intel-gfx] " Ville Syrjälä
2022-02-15 16:52                       ` Ville Syrjälä
2022-02-15 16:58                       ` [Intel-gfx] " Ville Syrjälä
2022-02-15 19:18                         ` Ville Syrjälä
2022-02-14  9:18 ` [Intel-gfx] [PATCH 3/6] drm/i915: Widen the QGV point mask Ville Syrjala
2022-02-14  9:18   ` Ville Syrjala
2022-02-14 17:17   ` Lisovskiy, Stanislav [this message]
2022-02-14 17:17     ` Lisovskiy, Stanislav
2022-02-14  9:18 ` [Intel-gfx] [PATCH 4/6] drm/i915: Unconfuse pre-icl vs. icl+ intel_sagv_{pre, post}_plane_update() Ville Syrjala
2022-02-14 17:39   ` Lisovskiy, Stanislav
2022-02-14  9:18 ` [Intel-gfx] [PATCH 5/6] drm/i915: Split pre-icl vs. icl+ SAGV hooks apart Ville Syrjala
2022-02-17 18:31   ` Lisovskiy, Stanislav
2022-02-14  9:18 ` [Intel-gfx] [PATCH 6/6] drm/i915: Pimp icl+ sagv pre/post update Ville Syrjala
2022-02-14 10:00   ` Lisovskiy, Stanislav
2022-02-14 10:27     ` Ville Syrjälä
2022-02-14 17:48       ` Lisovskiy, Stanislav
2022-02-14 18:04       ` Lisovskiy, Stanislav
2022-02-15 21:29 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: SAGV fixes Patchwork
2022-02-16  1:37 ` [Intel-gfx] ✗ Fi.CI.IGT: 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=20220214171732.GA25816@intel.com \
    --to=stanislav.lisovskiy@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=stable@vger.kernel.org \
    --cc=ville.syrjala@linux.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.