[Mesa-dev] [PATCH 2/3 v2] r600g: take into account offset to system inputs at tgsi_interp_egcm()

[Emil Velikov]

Hi Constantine,

Thanks for giving r600 some much needed love.

While the patch has landed, just going to share some generic comments.
Most of which are documented here https://www.mesa3d.org/submittingpatches.html.

On 24 June 2017 at 15:06, Constantine Kharlamov <Hi-Angel at yandex.ru> wrote:
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=100785
s/Fixes/Bugzilla/ and keep this above the s-o-b/r-b/other tags.

Should this fix land in the stable releases - see
https://www.mesa3d.org/submittingpatches.html#nominations for the
specifics.

>
> v2: I was too much twiddling whether to initialize nsys_inputs at the beginning of shader initialization or for allocation of system values, and by the time I decided to go with the first one, I forgot to change it back.
>
Mentioning in the commit message why you opted for the current
location might be a good idea?

In either way, please keep the text within ~72 columns.

> Signed-off-by: Constantine Kharlamov <Hi-Angel at yandex.ru>
> ---
>  src/gallium/drivers/r600/r600_shader.c | 8 ++++----
>  src/gallium/drivers/r600/r600_shader.h | 5 +++--
>  2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c
> index 156dba085d..2eb8187341 100644
> --- a/src/gallium/drivers/r600/r600_shader.c
> +++ b/src/gallium/drivers/r600/r600_shader.c
> @@ -1134,9 +1134,10 @@ static int allocate_system_value_inputs(struct r600_shader_ctx *ctx, int gpr_off

> -                       k = ctx->shader->ninput ++;
> +                       k = ctx->shader->ninput++;

[snip]

> -       unsigned        interpolate_location; //  TGSI_INTERPOLATE_LOC_CENTER, CENTROID, SAMPLE
> +       unsigned                interpolate_location; //  TGSI_INTERPOLATE_LOC_CENTER, CENTROID, SAMPLE
>         unsigned                lds_pos; /* for evergreen */
>         unsigned                back_color_input;
>         unsigned                write_mask;
> -       int                             ring_offset;
> +       int                     ring_offset;

These seem like unrelated white space changes. Please try to keep
those separate patches since they only distract from the important
parts of the commit.

-Emil