[Intel-gfx] [PATCH 04/13] drm/i915/dsb: Fix DSB command buffer size checks

[Manna, Animesh]

Hi,

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Friday, December 16, 2022 6:08 AM
> To: intel-gfx at lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 04/13] drm/i915/dsb: Fix DSB command buffer
> size checks
> 
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> free_pos is in dwords, DSB_BUF_SIZE in bytes. Directly comparing the two is
> nonsense. Fix it up, and make sure we also account for the 8byte alignment
> requirement for each instruction, and also assume that each instruction
> normally eats two dwords.

free_pos variable is used as index to fill the dsb command buffer and getting filed byte by byte. It is not in dwords.
Below given the structure definition of dsb cmd_buf.
struct intel_dsb {
        enum dsb_id id;

        u32 *cmd_buf;


> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dsb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
> b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 6abfd0fc541a..fbcbf9efd039 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -97,7 +97,7 @@ void intel_dsb_indexed_reg_write(struct intel_dsb
> *dsb,
>  	u32 *buf = dsb->cmd_buf;
>  	u32 reg_val;
> 
> -	if (drm_WARN_ON(&dev_priv->drm, dsb->free_pos >=
> DSB_BUF_SIZE)) {
> +	if (drm_WARN_ON(&dev_priv->drm, ALIGN(dsb->free_pos, 2) >
> DSB_BUF_SIZE
> +/ 4 - 2)) {

Here DSB_BUF_SIZE is 4096 x 2 = 8192 bytes
DSB_BUF_SIZE / 4 - 2 = 2046 bytes.
With the above logic warning will be triggered after 2046 bytes.

Regards,
Animesh


>  		drm_dbg_kms(&dev_priv->drm, "DSB buffer overflow\n");
>  		return;
>  	}
> @@ -167,7 +167,7 @@ void intel_dsb_reg_write(struct intel_dsb *dsb,
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	u32 *buf = dsb->cmd_buf;
> 
> -	if (drm_WARN_ON(&dev_priv->drm, dsb->free_pos >=
> DSB_BUF_SIZE)) {
> +	if (drm_WARN_ON(&dev_priv->drm, ALIGN(dsb->free_pos, 2) >
> DSB_BUF_SIZE
> +/ 4 - 2)) {
>  		drm_dbg_kms(&dev_priv->drm, "DSB buffer overflow\n");
>  		return;
>  	}
> --
> 2.37.4