[Mesa-dev] [PATCH 1/2] radeonsi: fix SPI_PS_INPUT_ENA handling

[Christian König]

On 23.08.2012 16:37, Michel Dänzer wrote:
> On Mit, 2012-08-22 at 12:54 +0200, Christian König wrote:
>> +	/* we need to enable at least one of them, otherwise we hang the GPU */
>> +	if (!spi_ps_input_ena & (C_0286CC_PERSP_SAMPLE_ENA |
>> +				 C_0286CC_PERSP_CENTROID_ENA |
>> +				 C_0286CC_PERSP_PULL_MODEL_ENA |
>> +				 C_0286CC_LINEAR_SAMPLE_ENA |
>> +				 C_0286CC_LINEAR_CENTER_ENA |
>> +				 C_0286CC_LINEAR_CENTROID_ENA |
>> +				 C_0286CC_LINE_STIPPLE_TEX_ENA)) {
>> +		spi_ps_input_ena |= S_0286CC_PERSP_SAMPLE_ENA(1);
>> +	}
> I just noticed that this causes a warning:
>
> si_state_draw.c: In function ‘si_pipe_shader_ps’:
> si_state_draw.c:179:6: warning: suggest parentheses around operand of ‘!’ or change ‘&’ to ‘&&’ or ‘!’ to ‘~’ [-Wparentheses]
>
> Did you mean the following?
>
> if (!(spi_ps_input_ena & (~C_0286CC_PERSP_SAMPLE_ENA |
>    ~C_0286CC_PERSP_CENTROID_ENA |
>    ~C_0286CC_PERSP_PULL_MODEL_ENA |
>    ~C_0286CC_LINEAR_SAMPLE_ENA |
>    ~C_0286CC_LINEAR_CENTER_ENA |
>    ~C_0286CC_LINEAR_CENTROID_ENA |
>    ~C_0286CC_LINE_STIPPLE_TEX_ENA)) {
> spi_ps_input_ena |= S_0286CC_PERSP_SAMPLE_ENA(1);
> }

Oh yes indeed. But lets use the G_* variants instead. I think they are 
better readable:

         if (!G_0286CC_PERSP_SAMPLE_ENA(spi_ps_input_ena) &&
             !G_0286CC_PERSP_CENTROID_ENA(spi_ps_input_ena) &&
             !G_0286CC_PERSP_PULL_MODEL_ENA(spi_ps_input_ena) &&
             !G_0286CC_LINEAR_SAMPLE_ENA(spi_ps_input_ena) &&
             !G_0286CC_LINEAR_CENTER_ENA(spi_ps_input_ena) &&
             !G_0286CC_LINEAR_CENTROID_ENA(spi_ps_input_ena) &&
             !G_0286CC_LINE_STIPPLE_TEX_ENA(spi_ps_input_ena)) {

                 spi_ps_input_ena |= S_0286CC_PERSP_SAMPLE_ENA(1);
         }

Should I go ahead and commit it like this?

Christian.