Re: [PATCH v7 2/3] drm/i915/gt: Do not generate the command streamer for all the CCS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 27, 2024 at 04:56:18PM +0100, Andi Shyti wrote:
> We want a fixed load CCS balancing consisting in all slices
> sharing one single user engine. For this reason do not create the
> intel_engine_cs structure with its dedicated command streamer for
> CCS slices beyond the first.
> 
> Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
> Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris.p.wilson@xxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> # v6.2+
> Acked-by: Michal Mrozek <michal.mrozek@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index f553cf4e6449..47c4a69e854c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -908,6 +908,21 @@ static intel_engine_mask_t init_engine_mask(struct intel_gt *gt)
>  		info->engine_mask &= ~BIT(GSC0);
>  	}
>  
> +	/*
> +	 * Do not create the command streamer for CCS slices beyond the first.
> +	 * All the workload submitted to the first engine will be shared among
> +	 * all the slices.
> +	 *
> +	 * Once the user will be allowed to customize the CCS mode, then this
> +	 * check needs to be removed.
> +	 */
> +	if (IS_DG2(gt->i915)) {
> +		intel_engine_mask_t first_ccs = BIT((CCS0 + __ffs(CCS_MASK(gt))));
> +		intel_engine_mask_t all_ccs = CCS_MASK(gt) << CCS0;
> +
> +		info->engine_mask &= ~(all_ccs &= ~first_ccs);

Shouldn't the second "&=" just be an "&" since there's no need to modify
the all_ccs variable that never gets used again?

In fact since this is DG2-specific, it seems like it might be more
intuitive to just write the whole thing more directly as

        if (IS_DG2(gt->i915)) {
                int first_ccs = __ffs(CCS_MASK(gt));

                info->engine_mask &= ~GENMASK(CCS3, CCS0);
                info->engine_mask |= BIT(_CCS(first_ccs));
        }

But up to you; if you just want to remove the unnecessary "=" that's
fine too.  Either way,

        Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>


Matt

> +	}
> +
>  	return info->engine_mask;
>  }
>  
> -- 
> 2.43.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux