[03/18] gallium/radeon: add a workaround when viewport state is used to force Z

Submitted by Marek Olšák on Aug. 17, 2017, 6:31 p.m.

Details

Message ID 1502994699-19837-4-git-send-email-maraeo@gmail.com
State New
Headers show
Series "Gallium blitter optimizations" ( rev: 3 2 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Marek Olšák Aug. 17, 2017, 6:31 p.m.
From: Marek Olšák <marek.olsak@amd.com>

This will be needed by u_blitter.
---
 src/gallium/drivers/radeon/r600_viewport.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/drivers/radeon/r600_viewport.c b/src/gallium/drivers/radeon/r600_viewport.c
index 2de1382..f7fc536 100644
--- a/src/gallium/drivers/radeon/r600_viewport.c
+++ b/src/gallium/drivers/radeon/r600_viewport.c
@@ -316,50 +316,69 @@  static void r600_emit_viewports(struct r600_common_context *rctx)
 		u_bit_scan_consecutive_range(&mask, &start, &count);
 
 		radeon_set_context_reg_seq(cs, R_02843C_PA_CL_VPORT_XSCALE +
 					       start * 4 * 6, count * 6);
 		for (i = start; i < start+count; i++)
 			r600_emit_one_viewport(rctx, &states[i]);
 	}
 	rctx->viewports.dirty_mask = 0;
 }
 
+static void r600_get_zmin_zmax(const struct pipe_viewport_state *vp,
+			       bool clip_halfz, float *zmin, float *zmax)
+{
+	/* For some reason, setting zmin == zmax breaks things.
+	 * Setting zmin = 0 && zmin = 1 works around it.
+	 * u_blitter sets scale = 0 && translate = Z to force a certain Z
+	 * value. The clamp would have no effect anyway, but it would be nice
+	 * to know what the problem is with zmin == zmax.
+	 */
+	if (vp->scale[2] == 0) {
+		assert(vp->translate[2] >= 0 && vp->translate[2] <= 1);
+		*zmin = 0;
+		*zmax = 1;
+		return;
+	}
+
+	util_viewport_zmin_zmax(vp, clip_halfz, zmin, zmax);
+}
+
 static void r600_emit_depth_ranges(struct r600_common_context *rctx)
 {
 	struct radeon_winsys_cs *cs = rctx->gfx.cs;
 	struct pipe_viewport_state *states = rctx->viewports.states;
 	unsigned mask = rctx->viewports.depth_range_dirty_mask;
 	float zmin, zmax;
 
 	/* The simple case: Only 1 viewport is active. */
 	if (!rctx->vs_writes_viewport_index) {
 		if (!(mask & 1))
 			return;
 
-		util_viewport_zmin_zmax(&states[0], rctx->clip_halfz, &zmin, &zmax);
+		r600_get_zmin_zmax(&states[0], rctx->clip_halfz, &zmin, &zmax);
 
 		radeon_set_context_reg_seq(cs, R_0282D0_PA_SC_VPORT_ZMIN_0, 2);
 		radeon_emit(cs, fui(zmin));
 		radeon_emit(cs, fui(zmax));
 		rctx->viewports.depth_range_dirty_mask &= ~1; /* clear one bit */
 		return;
 	}
 
 	while (mask) {
 		int start, count, i;
 
 		u_bit_scan_consecutive_range(&mask, &start, &count);
 
 		radeon_set_context_reg_seq(cs, R_0282D0_PA_SC_VPORT_ZMIN_0 +
 					   start * 4 * 2, count * 2);
 		for (i = start; i < start+count; i++) {
-			util_viewport_zmin_zmax(&states[i], rctx->clip_halfz, &zmin, &zmax);
+			r600_get_zmin_zmax(&states[i], rctx->clip_halfz, &zmin, &zmax);
 			radeon_emit(cs, fui(zmin));
 			radeon_emit(cs, fui(zmax));
 		}
 	}
 	rctx->viewports.depth_range_dirty_mask = 0;
 }
 
 static void r600_emit_viewport_states(struct r600_common_context *rctx,
 				      struct r600_atom *atom)
 {

Comments

On 17.08.2017 20:31, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak@amd.com>
> 
> This will be needed by u_blitter.
> ---
>   src/gallium/drivers/radeon/r600_viewport.c | 23 +++++++++++++++++++++--
>   1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/drivers/radeon/r600_viewport.c b/src/gallium/drivers/radeon/r600_viewport.c
> index 2de1382..f7fc536 100644
> --- a/src/gallium/drivers/radeon/r600_viewport.c
> +++ b/src/gallium/drivers/radeon/r600_viewport.c
> @@ -316,50 +316,69 @@ static void r600_emit_viewports(struct r600_common_context *rctx)
>   		u_bit_scan_consecutive_range(&mask, &start, &count);
>   
>   		radeon_set_context_reg_seq(cs, R_02843C_PA_CL_VPORT_XSCALE +
>   					       start * 4 * 6, count * 6);
>   		for (i = start; i < start+count; i++)
>   			r600_emit_one_viewport(rctx, &states[i]);
>   	}
>   	rctx->viewports.dirty_mask = 0;
>   }
>   
> +static void r600_get_zmin_zmax(const struct pipe_viewport_state *vp,
> +			       bool clip_halfz, float *zmin, float *zmax)
> +{
> +	/* For some reason, setting zmin == zmax breaks things.
> +	 * Setting zmin = 0 && zmin = 1 works around it.
> +	 * u_blitter sets scale = 0 && translate = Z to force a certain Z
> +	 * value. The clamp would have no effect anyway, but it would be nice
> +	 * to know what the problem is with zmin == zmax.

Weird rounding/floating point issues? Could the blitter just set scale = 
1 for the same effect? Seems like that would be less hacky...

Cheers,
Nicolai


> +	 */
> +	if (vp->scale[2] == 0) {
> +		assert(vp->translate[2] >= 0 && vp->translate[2] <= 1);
> +		*zmin = 0;
> +		*zmax = 1;
> +		return;
> +	}
> +
> +	util_viewport_zmin_zmax(vp, clip_halfz, zmin, zmax);
> +}
> +
>   static void r600_emit_depth_ranges(struct r600_common_context *rctx)
>   {
>   	struct radeon_winsys_cs *cs = rctx->gfx.cs;
>   	struct pipe_viewport_state *states = rctx->viewports.states;
>   	unsigned mask = rctx->viewports.depth_range_dirty_mask;
>   	float zmin, zmax;
>   
>   	/* The simple case: Only 1 viewport is active. */
>   	if (!rctx->vs_writes_viewport_index) {
>   		if (!(mask & 1))
>   			return;
>   
> -		util_viewport_zmin_zmax(&states[0], rctx->clip_halfz, &zmin, &zmax);
> +		r600_get_zmin_zmax(&states[0], rctx->clip_halfz, &zmin, &zmax);
>   
>   		radeon_set_context_reg_seq(cs, R_0282D0_PA_SC_VPORT_ZMIN_0, 2);
>   		radeon_emit(cs, fui(zmin));
>   		radeon_emit(cs, fui(zmax));
>   		rctx->viewports.depth_range_dirty_mask &= ~1; /* clear one bit */
>   		return;
>   	}
>   
>   	while (mask) {
>   		int start, count, i;
>   
>   		u_bit_scan_consecutive_range(&mask, &start, &count);
>   
>   		radeon_set_context_reg_seq(cs, R_0282D0_PA_SC_VPORT_ZMIN_0 +
>   					   start * 4 * 2, count * 2);
>   		for (i = start; i < start+count; i++) {
> -			util_viewport_zmin_zmax(&states[i], rctx->clip_halfz, &zmin, &zmax);
> +			r600_get_zmin_zmax(&states[i], rctx->clip_halfz, &zmin, &zmax);
>   			radeon_emit(cs, fui(zmin));
>   			radeon_emit(cs, fui(zmax));
>   		}
>   	}
>   	rctx->viewports.depth_range_dirty_mask = 0;
>   }
>   
>   static void r600_emit_viewport_states(struct r600_common_context *rctx,
>   				      struct r600_atom *atom)
>   {
>
On Fri, Aug 18, 2017 at 3:55 PM, Nicolai Hähnle <nhaehnle@gmail.com> wrote:
> On 17.08.2017 20:31, Marek Olšák wrote:
>>
>> From: Marek Olšák <marek.olsak@amd.com>
>>
>> This will be needed by u_blitter.
>> ---
>>   src/gallium/drivers/radeon/r600_viewport.c | 23 +++++++++++++++++++++--
>>   1 file changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeon/r600_viewport.c
>> b/src/gallium/drivers/radeon/r600_viewport.c
>> index 2de1382..f7fc536 100644
>> --- a/src/gallium/drivers/radeon/r600_viewport.c
>> +++ b/src/gallium/drivers/radeon/r600_viewport.c
>> @@ -316,50 +316,69 @@ static void r600_emit_viewports(struct
>> r600_common_context *rctx)
>>                 u_bit_scan_consecutive_range(&mask, &start, &count);
>>                 radeon_set_context_reg_seq(cs, R_02843C_PA_CL_VPORT_XSCALE
>> +
>>                                                start * 4 * 6, count * 6);
>>                 for (i = start; i < start+count; i++)
>>                         r600_emit_one_viewport(rctx, &states[i]);
>>         }
>>         rctx->viewports.dirty_mask = 0;
>>   }
>>   +static void r600_get_zmin_zmax(const struct pipe_viewport_state *vp,
>> +                              bool clip_halfz, float *zmin, float *zmax)
>> +{
>> +       /* For some reason, setting zmin == zmax breaks things.
>> +        * Setting zmin = 0 && zmin = 1 works around it.
>> +        * u_blitter sets scale = 0 && translate = Z to force a certain Z
>> +        * value. The clamp would have no effect anyway, but it would be
>> nice
>> +        * to know what the problem is with zmin == zmax.
>
>
> Weird rounding/floating point issues? Could the blitter just set scale = 1
> for the same effect? Seems like that would be less hacky...

Yes, scale = 1 works too and doesn't need the workaround.

Marek

>
> Cheers,
> Nicolai
>
>
>
>> +        */
>> +       if (vp->scale[2] == 0) {
>> +               assert(vp->translate[2] >= 0 && vp->translate[2] <= 1);
>> +               *zmin = 0;
>> +               *zmax = 1;
>> +               return;
>> +       }
>> +
>> +       util_viewport_zmin_zmax(vp, clip_halfz, zmin, zmax);
>> +}
>> +
>>   static void r600_emit_depth_ranges(struct r600_common_context *rctx)
>>   {
>>         struct radeon_winsys_cs *cs = rctx->gfx.cs;
>>         struct pipe_viewport_state *states = rctx->viewports.states;
>>         unsigned mask = rctx->viewports.depth_range_dirty_mask;
>>         float zmin, zmax;
>>         /* The simple case: Only 1 viewport is active. */
>>         if (!rctx->vs_writes_viewport_index) {
>>                 if (!(mask & 1))
>>                         return;
>>   -             util_viewport_zmin_zmax(&states[0], rctx->clip_halfz,
>> &zmin, &zmax);
>> +               r600_get_zmin_zmax(&states[0], rctx->clip_halfz, &zmin,
>> &zmax);
>>                 radeon_set_context_reg_seq(cs,
>> R_0282D0_PA_SC_VPORT_ZMIN_0, 2);
>>                 radeon_emit(cs, fui(zmin));
>>                 radeon_emit(cs, fui(zmax));
>>                 rctx->viewports.depth_range_dirty_mask &= ~1; /* clear one
>> bit */
>>                 return;
>>         }
>>         while (mask) {
>>                 int start, count, i;
>>                 u_bit_scan_consecutive_range(&mask, &start, &count);
>>                 radeon_set_context_reg_seq(cs, R_0282D0_PA_SC_VPORT_ZMIN_0
>> +
>>                                            start * 4 * 2, count * 2);
>>                 for (i = start; i < start+count; i++) {
>> -                       util_viewport_zmin_zmax(&states[i],
>> rctx->clip_halfz, &zmin, &zmax);
>> +                       r600_get_zmin_zmax(&states[i], rctx->clip_halfz,
>> &zmin, &zmax);
>>                         radeon_emit(cs, fui(zmin));
>>                         radeon_emit(cs, fui(zmax));
>>                 }
>>         }
>>         rctx->viewports.depth_range_dirty_mask = 0;
>>   }
>>     static void r600_emit_viewport_states(struct r600_common_context
>> *rctx,
>>                                       struct r600_atom *atom)
>>   {
>>
>
>
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.