Re: [PATCH 3/3] drm/xe/guc: Bump the G2H queue size to account for page faults

["Summers, Stuart" <stuart.summers@intel.com>]

On Fri, 2024-07-19 at 23:59 +0000, Matthew Brost wrote:
> On Fri, Jul 19, 2024 at 07:06:14PM +0000, Stuart Summers wrote:
> > With the increase in the size of the recoverable page fault
> > queue, we want to ensure the initial messages from GuC in
> > the G2H buffer have space while we transfer those out to the
> > actual pf_queue. Bump the G2H queue size to account for this
> > increase in the pf_queue size.
> > 
> 
> For future reference, include change log to help reviewers.

Sure makes sense. I had included the change log in the cover letter but
decided not to include here. I'll do that in the future.

> 
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>

Thanks for the reviews Matt!

> 
> > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_guc_ct.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c
> > b/drivers/gpu/drm/xe/xe_guc_ct.c
> > index 7d2e937da1d8..a3e9dd71f957 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> > @@ -105,12 +105,20 @@ ct_to_xe(struct xe_guc_ct *ct)
> >   * enough space to avoid backpressure on the driver. We increase
> > the size
> >   * of the receive buffer (relative to the send) to ensure a G2H
> > response
> >   * CTB has a landing spot.
> > + *
> > + * In addition to submissions, the G2H buffer needs to be able to
> > hold
> > + * enough space for recoverable page fault notifications. The
> > number of
> > + * page faults is interrupt driven and can be as much as the
> > number of
> > + * compute resources available. However, most of the actual work
> > for these
> > + * is in a separate page fault worker thread. Therefore we only
> > need to
> > + * make sure the queue has enough space to handle all of the
> > submissions
> > + * and responses and an extra buffer for incoming page faults.
> >   */
> >  
> >  #define CTB_DESC_SIZE          ALIGN(sizeof(struct
> > guc_ct_buffer_desc), SZ_2K)
> >  #define CTB_H2G_BUFFER_SIZE    (SZ_4K)
> > -#define CTB_G2H_BUFFER_SIZE    (4 * CTB_H2G_BUFFER_SIZE)
> > -#define G2H_ROOM_BUFFER_SIZE   (CTB_G2H_BUFFER_SIZE / 4)
> > +#define CTB_G2H_BUFFER_SIZE    (SZ_128K)
> > +#define G2H_ROOM_BUFFER_SIZE   (CTB_G2H_BUFFER_SIZE / 2)
> >  
> >  /**
> >   * xe_guc_ct_queue_proc_time_jiffies - Return maximum time to
> > process a full
> > -- 
> > 2.34.1
> >