Re: [Intel-gfx] [PATCH 1/3] drm/i915: add a dedicated workqueue inside drm_i915_private

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 11/05/2023 09:20, Luca Coelho wrote:
> In order to avoid flush_scheduled_work() usage, add a dedicated
> workqueue in the drm_i915_private structure.  In this way, we don't
> need to use the system queue anymore.
> 
> This change is mostly mechanical and based on Tetsuo's original
> patch[1].
> 
> Link: https://patchwork.freedesktop.org/series/114608/ [1]
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>

[snip]

> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 14c5338c96a6..06cd956b03ab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -259,6 +259,8 @@ struct drm_i915_private {
>   	 */
>   	struct workqueue_struct *wq;
>   
> +	struct workqueue_struct *i915_wq;

Two things.

First, the i915->wq has a nice big comments accompanying both the 
declaration and initialization. Lets come up with at least one solid 
comment for the new one too. No one will otherwise know when to use 
i915->wq and when i915->i915_wq.

Which leads me to the second point. Lets try find a nicer name for it. 
i915->i915_wq reads a bit bad when we both have i915->wq and also i915 
mentioned twice. Easy cheat option could be to signify the property in 
the name - like i915->unordered_wq. Or something. It may depend on what 
that "solid comment" from the first point will be. I mean what we decide 
the prescribed use cases for the new wq will be.

Regards,

Tvrtko