Re: [PATCH 2/2] drm/vc4: Add exec flags to allow forcing a specific X/Y tile walk order.
From: Eric Anholt
Date: Wed Aug 09 2017 - 13:38:50 EST
Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> writes:
> On Tue, 25 Jul 2017 09:27:33 -0700
> Eric Anholt <eric@xxxxxxxxxx> 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@xxxxxxxxxx>
>> ---
>>
>> 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@xxxxxxxxxxxxxxxxxx>
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!
Attachment:
signature.asc
Description: PGP signature