[Mesa-dev] [PATCH] i965/cnl: Add l3 configuration for Cannonlake

[Francisco Jerez]

Anuj Phogat <anuj.phogat at gmail.com> writes:

> From: Ben Widawsky <benjamin.widawsky at intel.com>
>
> V2 (Anuj):
> Squash the changes in one patch rebase on master.
> Address the review comments made by Francisco Jerez.

You seem to have missed half of my review comments, I'll repeat them
below for you.

> Do the URB allocation per slice (not per bank).
>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> Cc: Francisco Jerez <currojerez at riseup.net>
> ---
> V1 was sent out with the heading:
> "i965/cnl: Properly handle l3 configuration"
> ---
>  src/intel/common/gen_l3_config.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/common/gen_l3_config.c b/src/intel/common/gen_l3_config.c
> index 9a7771a..052c21b 100644
> --- a/src/intel/common/gen_l3_config.c
> +++ b/src/intel/common/gen_l3_config.c
> @@ -116,6 +116,24 @@ static const struct gen_l3_config bxt_2x6_l3_configs[] = {
>  };
>  
>  /**
> + * On CNL, the previous "IS" "C" and "T" fields do not exist in a
> + * programmable way. As a result we have fewer allocation parameters.
> + */

Comment is misleading, this change happened long before CNL.  Please
replace it with a comment along the lines of the headers of the other
L3 config tables.

> +static const struct gen_l3_config cnl_l3_configs[] = {
> +   /* SLM URB Rest DC RO */

Please format this consistently with the other L3 config tables, like:

|   /* SLM URB ALL DC  RO  IS   C   T */
|   {{ /* table goes here... */       }}

> +   {{  0, 64, 64,  0,  0 }},
> +   {{  0, 64,  0, 16, 48 }},
> +   {{  0, 48,  0, 16, 64 }},
> +   {{  0, 32,  0,  0, 96 }},
> +   {{  0, 32, 96,  0,  0 }},
> +   {{  0, 32,  0, 16, 80 }},
> +   {{ 32, 16, 80,  0,  0 }},
> +   {{ 32, 16,  0, 64, 16 }},
> +   {{ 32,  0, 96,  0,  0 }},
> +   {{ 0 }}
> +};
> +
> +/**
>   * Return a zero-terminated array of validated L3 configurations for the
>   * specified device.
>   */
> @@ -130,11 +148,13 @@ get_l3_configs(const struct gen_device_info *devinfo)
>        return (devinfo->is_cherryview ? chv_l3_configs : bdw_l3_configs);
>  
>     case 9:
> -   case 10:
>        if (devinfo->gen >= 9 && devinfo->l3_banks == 1)
>           return bxt_2x6_l3_configs;
>        return chv_l3_configs;
>  
> +   case 10:
> +      return cnl_l3_configs;
> +
>     default:
>        unreachable("Not implemented");
>     }
> -- 
> 2.9.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170619/f84c5fb5/attachment.sig>