On Mon, 09 May 2022 14:01:49 -0700, Alan Previn wrote:
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> index f454d53a8bca..35709202b09c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> @@ -15,7 +15,7 @@
>
> static void guc_log_copy_debuglogs_for_relay(struct intel_guc_log *log);
>
> -static u32 intel_guc_log_size(struct intel_guc_log *log)
> +u32 intel_guc_log_size(struct intel_guc_log *log)
> {
> /*
> * GuC Log buffer Layout:
> @@ -41,6 +41,12 @@ static u32 intel_guc_log_size(struct intel_guc_log *log)
> return PAGE_SIZE + CRASH_BUFFER_SIZE + DEBUG_BUFFER_SIZE + CAPTURE_BUFFER_SIZE;
> }
>
> +#define GUC_LOG_RELAY_SUBBUF_COUNT 8
> +u32 intel_guc_log_relay_subbuf_count(struct intel_guc_log *log)
> +{
> + return GUC_LOG_RELAY_SUBBUF_COUNT;
> +}
uapi wise, instead of exposing guc_log_size and subbuf_count, why not
expose subbuf_size and subbuf_count?
> +static int guc_log_relay_buf_size_get(void *data, u64 *val)
> +{
> + struct intel_guc_log *log = data;
> +
> + if (!log)
> + return -ENODEV;
> + if (!log->vma)
> + return -ENODEV;
Why are these checks needed? Hasn't log been initialized and attached at
intel_gt_debugfs_register_files()/debugfs_create_file() time itself?
Also if needed let's do:
if (!log || !log->vma)
return -ENODEV;
> +
> + *val = (u64)intel_guc_log_size(log);
> +
> + return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(guc_log_relay_buf_size_fops,
> + guc_log_relay_buf_size_get, NULL,
> + "%lld\n");
> +
> +static int guc_log_relay_subbuf_count_get(void *data, u64 *val)
> +{
> + struct intel_guc_log *log = data;
> +
> + if (!log)
> + return -ENODEV;
> + if (!log->vma)
> + return -ENODEV;
Same issue as above.
> +
> + *val = intel_guc_log_relay_subbuf_count(log);
> +
> + return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(guc_log_relay_subbuf_count_fops,
> + guc_log_relay_subbuf_count_get, NULL,
> + "%lld\n");
> +
> static int guc_log_relay_open(struct inode *inode, struct file *file)
> {
> struct intel_guc_log *log = inode->i_private;
>
> + if (!log)
> + return -ENODEV;
> +
Same issue as above.
> if (!intel_guc_is_ready(log_to_guc(log)))
> return -ENODEV;
>
> @@ -166,6 +205,8 @@ void intel_guc_log_debugfs_register(struct intel_guc_log *log,
> { "guc_load_err_log_dump", &guc_load_err_log_dump_fops, NULL },
> { "guc_log_level", &guc_log_level_fops, NULL },
> { "guc_log_relay", &guc_log_relay_fops, NULL },
> + { "guc_log_relay_buf_size", &guc_log_relay_buf_size_fops, NULL },
> + { "guc_log_relay_subbuf_count", &guc_log_relay_subbuf_count_fops, NULL },
> };
>
> if (!intel_guc_is_supported(log_to_guc(log)))
> --
> 2.25.1
>
