[Intel-gfx] [PATCH v2 1/1] drm/i915/guc: Delay disabling guc_id scheduling for better hysteresis

Teres Alexis, Alan Previn alan.previn.teres.alexis at intel.com
Thu Oct 6 05:43:19 UTC 2022



On Wed, 2022-10-05 at 17:25 -0700, Harrison, John C wrote:
> On 9/21/2022 10:32, Alan Previn wrote:
> > @@ -208,6 +210,11 @@ struct intel_context {
> >   		 * each priority bucket
> >   		 */
> >   		u32 prio_count[GUC_CLIENT_PRIORITY_NUM];
> > +		/**
> > +		 * @sched_disable_delay: worker to disable scheduling on this
> > +		 * context
> > +		 */
> > +		struct delayed_work sched_disable_delay;
> Nit: this confuses me every time that it looks like the delay timeout 
> rather than the worker object. It would be much easier to read the code 
> if it was 'sched_disable_delay_work', although that is quite the 
> mouthful. Maybe just call all three variables disable_delay_XXX?

I agree with you - I've modified this patch many times and realize I
prefer something self-explanatory even if its a mouthful.
Will stick with your first proposal "sched_disable_delay_work"

> 
> > -	if (unlikely(!context_enabled(ce) || submission_disabled(guc) ||
> > -		     context_has_committed_requests(ce))) {
> This function no longer has any callers so can be removed completely.
> 
Will do.

> Otherwise, it looks good to me.
> 
> John.
> 
> 



More information about the Intel-gfx mailing list