Re: [PATCH 01/10] drm/etnaviv: add uapi for register read feature

[Lucas Stach <l.stach@pengutronix.de>]

Hi Christian,

Am Freitag, den 09.12.2016, 12:21 +0100 schrieb Christian Gmeiner:
> We need to readout some registers _after_ the submited command
> buffer got executed in order to support perf counters.
> There is no way to read register via command stream - even the
> Vivante kernel driver does it via a special ioctl.
> 
> Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> ---
>  include/uapi/drm/etnaviv_drm.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
> index 2584c1c..0d30604 100644
> --- a/include/uapi/drm/etnaviv_drm.h
> +++ b/include/uapi/drm/etnaviv_drm.h
> @@ -150,6 +150,13 @@ struct drm_etnaviv_gem_submit_bo {
>  	__u64 presumed;       /* in/out, presumed buffer address */
>  };
>  
> +struct drm_etnaviv_gem_submit_readback {
> +	__u32 readback_offset;/* in, offset from readback_bo */
> +	__u32 readback_idx;   /* in, index of readback_bo buffer */

Why do we need this readback BO? Wouldn't it be easier to just have the
space for the readback value allocated in this struct and have the
kernel fill it in? We are reading the values by using the CPU anyways,
so I don't see the need for this indirection with a BO. We removed the
command BO with the same justification.

Regards,
Lucas

> +	__u32 reg;            /* in, register to read */
> +	__u32 flags;          /* in, needs to be 0 */
> +};
> +
>  /* Each cmdstream submit consists of a table of buffers involved, and
>   * one or more cmdstream buffers.  This allows for conditional execution
>   * (context-restore), and IB buffers needed for per tile/bin draw cmds.
> @@ -167,6 +174,9 @@ struct drm_etnaviv_gem_submit {
>  	__u64 bos;            /* in, ptr to array of submit_bo's */
>  	__u64 relocs;         /* in, ptr to array of submit_reloc's */
>  	__u64 stream;         /* in, ptr to cmdstream */
> +	__u64 readbacks;      /* in, ptr to array of submit_readback's */
> +	__u32 nr_readbacks;   /* in, number of submit_readback's */
> +	__u32 padding;
>  };
>  
>  /* The normal way to synchronize with the GPU is just to CPU_PREP on


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel