linux-kernel - Re: [PATCH 2/2] drm/vc4: Add exec flags to allow forcing a specific X/Y tile walk order.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87d185g5zb.fsf@eliezer.anholt.net>
Date:   Tue, 08 Aug 2017 13:27:36 -0700
From:   Eric Anholt <eric@...olt.net>
To:     Boris Brezillon <boris.brezillon@...e-electrons.com>
Cc:     dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] drm/vc4: Add exec flags to allow forcing a specific X/Y tile walk order.

Boris Brezillon <boris.brezillon@...e-electrons.com> writes:

> On Tue, 25 Jul 2017 09:27:33 -0700
> Eric Anholt <eric@...olt.net> wrote:
>
>> This is useful to allow GL to provide defined results for overlapping
>> glBlitFramebuffer, which X11 in turn uses to accelerate uncomposited
>> window movement without first blitting to a temporary.  x11perf
>> -copywinwin100 goes from 1850/sec to 4850/sec.
>> 
>> Signed-off-by: Eric Anholt <eric@...olt.net>
>> ---
>> 
>> The work-in-progress userspace is at:
>> 
>> https://github.com/anholt/xserver/commits/glamor-draw-bounds-overlap
>> https://github.com/anholt/mesa/commits/vc4-overlapping-blit
>> 
>> and the next step is to build the GL extension spec and piglit tests
>> for it.
>> 
>>  drivers/gpu/drm/vc4/vc4_drv.c       |  1 +
>>  drivers/gpu/drm/vc4/vc4_gem.c       |  5 ++++-
>>  drivers/gpu/drm/vc4/vc4_render_cl.c | 21 ++++++++++++++++-----
>>  include/uapi/drm/vc4_drm.h          | 11 +++++++++++
>>  4 files changed, 32 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
>> index c6b487c3d2b7..b5c2c28289ed 100644
>> --- a/drivers/gpu/drm/vc4/vc4_drv.c
>> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
>> @@ -99,6 +99,7 @@ static int vc4_get_param_ioctl(struct drm_device *dev, void *data,
>>  	case DRM_VC4_PARAM_SUPPORTS_BRANCHES:
>>  	case DRM_VC4_PARAM_SUPPORTS_ETC1:
>>  	case DRM_VC4_PARAM_SUPPORTS_THREADED_FS:
>> +	case DRM_VC4_PARAM_SUPPORTS_FIXED_RCL_ORDER:
>>  		args->value = true;
>>  		break;
>>  	default:
>> diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
>> index a3e45e67f417..ba0782ebda34 100644
>> --- a/drivers/gpu/drm/vc4/vc4_gem.c
>> +++ b/drivers/gpu/drm/vc4/vc4_gem.c
>> @@ -1008,7 +1008,10 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data,
>>  	struct ww_acquire_ctx acquire_ctx;
>>  	int ret = 0;
>>  
>> -	if ((args->flags & ~VC4_SUBMIT_CL_USE_CLEAR_COLOR) != 0) {
>> +	if ((args->flags & ~(VC4_SUBMIT_CL_USE_CLEAR_COLOR |
>> +			     VC4_SUBMIT_CL_FIXED_RCL_ORDER |
>> +			     VC4_SUBMIT_CL_RCL_ORDER_INCREASING_X |
>> +			     VC4_SUBMIT_CL_RCL_ORDER_INCREASING_Y)) != 0) {
>>  		DRM_DEBUG("Unknown flags: 0x%02x\n", args->flags);
>>  		return -EINVAL;
>>  	}
>> diff --git a/drivers/gpu/drm/vc4/vc4_render_cl.c b/drivers/gpu/drm/vc4/vc4_render_cl.c
>> index da3bfd53f0bd..c3b064052147 100644
>> --- a/drivers/gpu/drm/vc4/vc4_render_cl.c
>> +++ b/drivers/gpu/drm/vc4/vc4_render_cl.c
>> @@ -261,8 +261,17 @@ static int vc4_create_rcl_bo(struct drm_device *dev, struct vc4_exec_info *exec,
>>  	uint8_t max_y_tile = args->max_y_tile;
>>  	uint8_t xtiles = max_x_tile - min_x_tile + 1;
>>  	uint8_t ytiles = max_y_tile - min_y_tile + 1;
>> -	uint8_t x, y;
>> +	uint8_t xi, yi;
>>  	uint32_t size, loop_body_size;
>> +	bool positive_x = false;
>> +	bool positive_y = false;
>> +
>> +	if (args->flags & VC4_SUBMIT_CL_FIXED_RCL_ORDER) {
>> +		if (args->flags & VC4_SUBMIT_CL_RCL_ORDER_INCREASING_X)
>> +			positive_x = true;
>> +		if (args->flags & VC4_SUBMIT_CL_RCL_ORDER_INCREASING_Y)
>> +			positive_y = true;
>> +	}
>
> Are you sure you want the default value of positive_x/y to be false? It
> seems to me that before this patch you were always iterating in
> ascending order, but now, when VC4_SUBMIT_CL_FIXED_RCL_ORDER is not
> set you do the opposite. Maybe you really want to change the default
> behavior, just wanted to point this out.
>
> Otherwise,
>
> Reviewed-by: Boris Brezillon <boris.brezillon@...e-electrons.com>

I was undecided as well, but if you also thought it was funny to change
the default, that's convinced me to keep it the same.

Thanks!

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)