Re: [Intel-gfx] [PATCH v2 3/5] drm/i915/guc: Provide debugfs for log relay sub-buf info

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Tue, 06 Dec 2022 01:20:58 -0800, Alan Previn wrote:
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log_debugfs.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log_debugfs.c
> index ddfbe334689f..27756640338e 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log_debugfs.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log_debugfs.c
> @@ -105,6 +105,38 @@ DEFINE_SIMPLE_ATTRIBUTE(guc_log_level_fops,
>			guc_log_level_get, guc_log_level_set,
>			"%lld\n");
>
> +static int guc_log_relay_subbuf_size_get(void *data, u64 *val)
> +{
> +	struct intel_guc_log *log = data;
> +
> +	if (!log->vma)
> +		return -ENODEV;

For the record, from the other email thread, the issue here is whether this
check is needed.

Also, the issue is what happens if the relay is open and we unload the
module, what happens?

> +
> +	*val = (u64)intel_guc_log_size(log);

Don't cast, shouldn't need it.

> +
> +	return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(guc_log_relay_subbuf_size_fops,
> +			guc_log_relay_subbuf_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->vma)
> +		return -ENODEV;

Same for this check too.

> +
> +	*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;
> @@ -166,6 +198,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_subbuf_size", &guc_log_relay_subbuf_size_fops, NULL },
> +		{ "guc_log_relay_subbuf_count", &guc_log_relay_subbuf_count_fops, NULL },
>	};
>
>	if (!intel_guc_is_supported(log_to_guc(log)))

Thanks.
--
Ashutosh