Re: [PATCH v3] drm/xe/guc: Update GuC log buffer type value

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

On Thu, 2025-09-25 at 11:48 -0400, Zhanjun Dong wrote:
> Update GuC log buffer type value, to align with the GuC
> specification.
> 
> Signed-off-by: Zhanjun Dong <zhanjun.dong@intel.com>
> ---
> Change list:
> v3: Update comments
> v2: Use SZ_4K, instead of PAGE_SIZE
>     Expand for loop with switch and fallthrough
> ---
>  drivers/gpu/drm/xe/abi/guc_log_abi.h |  2 +-
>  drivers/gpu/drm/xe/xe_guc_log.c      | 24 ++++++++++++++----------
>  2 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/abi/guc_log_abi.h
> b/drivers/gpu/drm/xe/abi/guc_log_abi.h
> index 554630b7ccd9..b1819679fa35 100644
> --- a/drivers/gpu/drm/xe/abi/guc_log_abi.h
> +++ b/drivers/gpu/drm/xe/abi/guc_log_abi.h
> @@ -10,8 +10,8 @@
>  
>  /* GuC logging buffer types */
>  enum guc_log_buffer_type {
> -       GUC_LOG_BUFFER_CRASH_DUMP,
>         GUC_LOG_BUFFER_DEBUG,
> +       GUC_LOG_BUFFER_CRASH_DUMP,

Looks good - verified this matches the spec.

>         GUC_LOG_BUFFER_CAPTURE,
>  };
>  
> diff --git a/drivers/gpu/drm/xe/xe_guc_log.c
> b/drivers/gpu/drm/xe/xe_guc_log.c
> index c01ccb35dc75..9017abf112bf 100644
> --- a/drivers/gpu/drm/xe/xe_guc_log.c
> +++ b/drivers/gpu/drm/xe/xe_guc_log.c
> @@ -50,15 +50,15 @@ static size_t guc_log_size(void)
>          *  |     Capture state header      |
>          *  +-------------------------------+ 96B
>          *  |                               |
> -        *  +===============================+ PAGE_SIZE (4KB)
> -        *  |        Crash Dump logs        |
> -        *  +===============================+ + CRASH_SIZE
> +        *  +===============================+ 4KB
>          *  |          Debug logs           |
>          *  +===============================+ + DEBUG_SIZE
> +        *  |        Crash Dump logs        |
> +        *  +===============================+ + CRASH_SIZE
>          *  |         Capture logs          |
>          *  +===============================+ + CAPTURE_SIZE
>          */
> -       return PAGE_SIZE + CRASH_BUFFER_SIZE + DEBUG_BUFFER_SIZE +
> +       return SZ_4K + DEBUG_BUFFER_SIZE + CRASH_BUFFER_SIZE +
>                 CAPTURE_BUFFER_SIZE;
>  }
>  
> @@ -327,13 +327,17 @@ u32 xe_guc_get_log_buffer_size(struct
> xe_guc_log *log, enum guc_log_buffer_type
>   */
>  u32 xe_guc_get_log_buffer_offset(struct xe_guc_log *log, enum
> guc_log_buffer_type type)
>  {
> -       enum guc_log_buffer_type i;
> -       u32 offset = PAGE_SIZE;/* for the log_buffer_states */
> +       u32 offset = SZ_4K; /* 1st section size */

Agreed.

>  
> -       for (i = GUC_LOG_BUFFER_CRASH_DUMP; i <
> GUC_LOG_BUFFER_TYPE_MAX; ++i) {
> -               if (i == type)
> -                       break;
> -               offset += xe_guc_get_log_buffer_size(log, i);
> +       switch (type) {
> +       case GUC_LOG_BUFFER_CAPTURE:
> +               offset += xe_guc_get_log_buffer_size(log,
> GUC_LOG_BUFFER_CRASH_DUMP);
> +               fallthrough;
> +       case GUC_LOG_BUFFER_CRASH_DUMP:
> +               offset += xe_guc_get_log_buffer_size(log,
> GUC_LOG_BUFFER_DEBUG);
> +               fallthrough;
> +       case GUC_LOG_BUFFER_DEBUG:
> +               break;

Take a quick look at the CI result before merging, but otherwise these
changes look good:
Reviewed-by: Stuart Summers <stuart.summers@intel.com>

>         }
>  
>         return offset;