Re: [yavta PATCH 1/1] cache maintenance skip feature

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sakari,

Thank you for the patch.

On Saturday 23 January 2016 23:49:54 Sakari Ailus wrote:
> From: Samu Onkalo <samu.onkalo@intel.com>
> 
> Add options to skip cache maintenance.
> --dqbuf-skip-cache
> --qbuf-skip-cache

I wonder whether we should add support for this feature to yavta when it 
hasn't really been exercised much on the kernel side. I've been pondering for 
some time whether cache management shouldn't be handled in an implicit way 
instead, by letting the kernel skip cache handling when no userspace mapping 
exists instead of passing flags explicitly. I'd like to discuss and experiment 
with that approach first.

> Signed-off-by: Samu Onkalo <samu.onkalo@intel.com>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> This patch depends on my earlier patch "Fix --data-prefix option
> documentation".
> 
>  yavta.c |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/yavta.c b/yavta.c
> index b21c3b3..ec4acd6 100644
> --- a/yavta.c
> +++ b/yavta.c
> @@ -73,6 +73,8 @@ struct device
>  	unsigned int width;
>  	unsigned int height;
>  	uint32_t buffer_output_flags;
> +	uint32_t buffer_qbuf_flags;
> +	uint32_t buffer_dqbuf_flags;
>  	uint32_t timestamp_type;
> 
>  	unsigned char num_planes;
> @@ -1010,6 +1012,8 @@ static int video_queue_buffer(struct device *dev, int
> index, enum buffer_fill_mo }
>  	}
> 
> +	buf.flags |= dev->buffer_qbuf_flags;
> +
>  	if (video_is_mplane(dev)) {
>  		buf.m.planes = planes;
>  		buf.length = dev->num_planes;
> @@ -1662,6 +1666,7 @@ static int video_do_capture(struct device *dev,
> unsigned int nframes, buf.memory = dev->memtype;
>  		buf.length = VIDEO_MAX_PLANES;
>  		buf.m.planes = planes;
> +		buf.flags = dev->buffer_dqbuf_flags;
> 
>  		ret = ioctl(dev->fd, VIDIOC_DQBUF, &buf);
>  		if (ret < 0) {
> @@ -1781,6 +1786,7 @@ static void usage(const char *argv0)
>  	printf("-u, --userptr			Use the user pointers streaming 
method\n");
>  	printf("-w, --set-control 'ctrl value'	Set control 'ctrl' to 
'value'\n");
>  	printf("    --data-prefix		Write portions of buffer data before
> data_offset\n"); +	printf("    --[d]qbuf-skip-cache        Skip cache
> maintenance\n"); printf("    --buffer-size		Buffer size in bytes\n");
>  	printf("    --enum-formats		Enumerate formats\n");
>  	printf("    --enum-inputs		Enumerate inputs\n");
> @@ -1814,6 +1820,8 @@ static void usage(const char *argv0)
>  #define OPT_PREMULTIPLIED	269
>  #define OPT_QUEUE_LATE		270
>  #define OPT_DATA_PREFIX		271
> +#define OPT_DQBUF_NO_CACHE      272
> +#define OPT_QBUF_NO_CACHE	273
> 
>  static struct option opts[] = {
>  	{"buffer-size", 1, 0, OPT_BUFFER_SIZE},
> @@ -1822,6 +1830,7 @@ static struct option opts[] = {
>  	{"check-overrun", 0, 0, 'C'},
>  	{"data-prefix", 0, 0, OPT_DATA_PREFIX},
>  	{"delay", 1, 0, 'd'},
> +	{"dqbuf-skip-cache", 0, 0, OPT_DQBUF_NO_CACHE},
>  	{"enum-formats", 0, 0, OPT_ENUM_FORMATS},
>  	{"enum-inputs", 0, 0, OPT_ENUM_INPUTS},
>  	{"fd", 1, 0, OPT_FD},
> @@ -1838,6 +1847,7 @@ static struct option opts[] = {
>  	{"offset", 1, 0, OPT_USERPTR_OFFSET},
>  	{"pause", 0, 0, 'p'},
>  	{"premultiplied", 0, 0, OPT_PREMULTIPLIED},
> +	{"qbuf-skip-cache", 0, 0, OPT_QBUF_NO_CACHE},
>  	{"quality", 1, 0, 'q'},
>  	{"queue-late", 0, 0, OPT_QUEUE_LATE},
>  	{"get-control", 1, 0, 'r'},
> @@ -2089,6 +2099,12 @@ int main(int argc, char *argv[])
>  		case OPT_DATA_PREFIX:
>  			dev.write_data_prefix = true;
>  			break;
> +		case OPT_DQBUF_NO_CACHE:
> +			dev.buffer_dqbuf_flags |= V4L2_BUF_FLAG_NO_CACHE_INVALIDATE;
> +			break;
> +		case OPT_QBUF_NO_CACHE:
> +			dev.buffer_qbuf_flags |= V4L2_BUF_FLAG_NO_CACHE_INVALIDATE;
> +			break;
>  		default:
>  			printf("Invalid option -%c\n", c);
>  			printf("Run %s -h for help.\n", argv[0]);

-- 
Regards,

Laurent Pinchart