Re: [Mesa-dev] [PATCH] i965/skl/gt4: Fix URB programming restriction.

2015-11-09 Thread Ben Widawsky
On Mon, Nov 09, 2015 at 11:50:25AM -0800, Kenneth Graunke wrote:
> On Saturday, November 07, 2015 08:40:51 AM Ben Widawsky wrote:
> > On Fri, Nov 06, 2015 at 07:29:18PM -0800, Kenneth Graunke wrote:
> > > On Friday, November 06, 2015 06:12:27 PM Ben Widawsky wrote:
> > > > The comment in the code details the restriction. Thanks to Ken for 
> > > > having a very
> > > > helpful conversation with me, and spotting the blurb in the link I sent 
> > > > him :P.
> > > > 
> > > > There are still stability problems for me on GT4, but this definitely 
> > > > helps with
> > > > some of the failures.
> > > > 
> > > > Cc: Kenneth Graunke 
> > > > Cc: Jordan Justen 
> > > > Cc: mesa-sta...@lists.freedesktop.org (if the original gt4 patch goes 
> > > > to stable)
> > > > ---
> > > > 
> > > > Sarah, you should check this on KBL.
> > > > Cc: Sarah Sharp 
> > > > ---
> > > >  src/mesa/drivers/dri/i965/brw_device_info.c | 8 
> > > >  1 file changed, 8 insertions(+)
> > > > 
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_device_info.c 
> > > > b/src/mesa/drivers/dri/i965/brw_device_info.c
> > > > index 2ebc084..6117fd3 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_device_info.c
> > > > +++ b/src/mesa/drivers/dri/i965/brw_device_info.c
> > > > @@ -337,6 +337,14 @@ static const struct brw_device_info 
> > > > brw_device_info_skl_gt3 = {
> > > >  
> > > >  static const struct brw_device_info brw_device_info_skl_gt4 = {
> > > > GEN9_FEATURES, .gt = 4,
> > > > +   /* From the docs:
> > > > +*   URB is limited to 1008KB due to programming restrictions. This 
> > > > is not a
> > > > +*   restriction of the L3 implementation, but of the FF and other 
> > > > clients.
> > > > +*   Therefore, in a GT4 implementation it is possible for the 
> > > > programmed
> > > > +*   allocation of the L3 data array to provide 3*384KB=1152MB 
> > > > [sic] for URB,
> > > > +*   but only 1008KB of this will be used.
> > > > +*/
> > > 
> > > We should at least say which page/section this comes from (the section
> > > name exists in the Broadwell PRM, so it's not particularly secret).
> > > 
> > > Please put the text in quotes too.  My suggested formatting:
> > > 
> > >/* From the "L3 Allocation and Programming" documentation:
> > > *
> > > * "URB is limited to 1008KB due to programming restrictions.  This
> > > *  is not a restriction of the L3 implementation, but of the FF and
> > > *  other clients.  Therefore, in a GT4 implementation it is
> > > *  possible for the programmed allocation of the L3 data array to
> > > *  provide 3*384KB=1152MB [sic] for URB, but only 1008KB of this
> > > *  will be used."
> > > */
> > > 
> > > Regardless,
> > > Reviewed-by: Kenneth Graunke 
> > > 
> > 
> > Thanks! I had no idea it was in the BDW PRMs.
> 
> You can change "1152MB [sic]" to "1152KB" as they've fixed the docs :)
> 
> --Ken

That was fast. I'm working on the next hang before I push this, but I will
update it locally now.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965/skl/gt4: Fix URB programming restriction.

2015-11-09 Thread Kenneth Graunke
On Saturday, November 07, 2015 08:40:51 AM Ben Widawsky wrote:
> On Fri, Nov 06, 2015 at 07:29:18PM -0800, Kenneth Graunke wrote:
> > On Friday, November 06, 2015 06:12:27 PM Ben Widawsky wrote:
> > > The comment in the code details the restriction. Thanks to Ken for having 
> > > a very
> > > helpful conversation with me, and spotting the blurb in the link I sent 
> > > him :P.
> > > 
> > > There are still stability problems for me on GT4, but this definitely 
> > > helps with
> > > some of the failures.
> > > 
> > > Cc: Kenneth Graunke 
> > > Cc: Jordan Justen 
> > > Cc: mesa-sta...@lists.freedesktop.org (if the original gt4 patch goes to 
> > > stable)
> > > ---
> > > 
> > > Sarah, you should check this on KBL.
> > > Cc: Sarah Sharp 
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_device_info.c | 8 
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/brw_device_info.c 
> > > b/src/mesa/drivers/dri/i965/brw_device_info.c
> > > index 2ebc084..6117fd3 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_device_info.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_device_info.c
> > > @@ -337,6 +337,14 @@ static const struct brw_device_info 
> > > brw_device_info_skl_gt3 = {
> > >  
> > >  static const struct brw_device_info brw_device_info_skl_gt4 = {
> > > GEN9_FEATURES, .gt = 4,
> > > +   /* From the docs:
> > > +*   URB is limited to 1008KB due to programming restrictions. This 
> > > is not a
> > > +*   restriction of the L3 implementation, but of the FF and other 
> > > clients.
> > > +*   Therefore, in a GT4 implementation it is possible for the 
> > > programmed
> > > +*   allocation of the L3 data array to provide 3*384KB=1152MB [sic] 
> > > for URB,
> > > +*   but only 1008KB of this will be used.
> > > +*/
> > 
> > We should at least say which page/section this comes from (the section
> > name exists in the Broadwell PRM, so it's not particularly secret).
> > 
> > Please put the text in quotes too.  My suggested formatting:
> > 
> >/* From the "L3 Allocation and Programming" documentation:
> > *
> > * "URB is limited to 1008KB due to programming restrictions.  This
> > *  is not a restriction of the L3 implementation, but of the FF and
> > *  other clients.  Therefore, in a GT4 implementation it is
> > *  possible for the programmed allocation of the L3 data array to
> > *  provide 3*384KB=1152MB [sic] for URB, but only 1008KB of this
> > *  will be used."
> > */
> > 
> > Regardless,
> > Reviewed-by: Kenneth Graunke 
> > 
> 
> Thanks! I had no idea it was in the BDW PRMs.

You can change "1152MB [sic]" to "1152KB" as they've fixed the docs :)

--Ken


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965/skl/gt4: Fix URB programming restriction.

2015-11-07 Thread Ben Widawsky
On Fri, Nov 06, 2015 at 07:29:18PM -0800, Kenneth Graunke wrote:
> On Friday, November 06, 2015 06:12:27 PM Ben Widawsky wrote:
> > The comment in the code details the restriction. Thanks to Ken for having a 
> > very
> > helpful conversation with me, and spotting the blurb in the link I sent him 
> > :P.
> > 
> > There are still stability problems for me on GT4, but this definitely helps 
> > with
> > some of the failures.
> > 
> > Cc: Kenneth Graunke 
> > Cc: Jordan Justen 
> > Cc: mesa-sta...@lists.freedesktop.org (if the original gt4 patch goes to 
> > stable)
> > ---
> > 
> > Sarah, you should check this on KBL.
> > Cc: Sarah Sharp 
> > ---
> >  src/mesa/drivers/dri/i965/brw_device_info.c | 8 
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_device_info.c 
> > b/src/mesa/drivers/dri/i965/brw_device_info.c
> > index 2ebc084..6117fd3 100644
> > --- a/src/mesa/drivers/dri/i965/brw_device_info.c
> > +++ b/src/mesa/drivers/dri/i965/brw_device_info.c
> > @@ -337,6 +337,14 @@ static const struct brw_device_info 
> > brw_device_info_skl_gt3 = {
> >  
> >  static const struct brw_device_info brw_device_info_skl_gt4 = {
> > GEN9_FEATURES, .gt = 4,
> > +   /* From the docs:
> > +*   URB is limited to 1008KB due to programming restrictions. This is 
> > not a
> > +*   restriction of the L3 implementation, but of the FF and other 
> > clients.
> > +*   Therefore, in a GT4 implementation it is possible for the 
> > programmed
> > +*   allocation of the L3 data array to provide 3*384KB=1152MB [sic] 
> > for URB,
> > +*   but only 1008KB of this will be used.
> > +*/
> 
> We should at least say which page/section this comes from (the section
> name exists in the Broadwell PRM, so it's not particularly secret).
> 
> Please put the text in quotes too.  My suggested formatting:
> 
>/* From the "L3 Allocation and Programming" documentation:
> *
> * "URB is limited to 1008KB due to programming restrictions.  This
> *  is not a restriction of the L3 implementation, but of the FF and
> *  other clients.  Therefore, in a GT4 implementation it is
> *  possible for the programmed allocation of the L3 data array to
> *  provide 3*384KB=1152MB [sic] for URB, but only 1008KB of this
> *  will be used."
> */
> 
> Regardless,
> Reviewed-by: Kenneth Graunke 
> 

Thanks! I had no idea it was in the BDW PRMs.

> > +   .urb.size = 1008 / 3,
> >  };
> >  
> >  static const struct brw_device_info brw_device_info_bxt = {
> > 



> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] i965/skl/gt4: Fix URB programming restriction.

2015-11-06 Thread Ben Widawsky
The comment in the code details the restriction. Thanks to Ken for having a very
helpful conversation with me, and spotting the blurb in the link I sent him :P.

There are still stability problems for me on GT4, but this definitely helps with
some of the failures.

Cc: Kenneth Graunke 
Cc: Jordan Justen 
Cc: mesa-sta...@lists.freedesktop.org (if the original gt4 patch goes to stable)
---

Sarah, you should check this on KBL.
Cc: Sarah Sharp 
---
 src/mesa/drivers/dri/i965/brw_device_info.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_device_info.c 
b/src/mesa/drivers/dri/i965/brw_device_info.c
index 2ebc084..6117fd3 100644
--- a/src/mesa/drivers/dri/i965/brw_device_info.c
+++ b/src/mesa/drivers/dri/i965/brw_device_info.c
@@ -337,6 +337,14 @@ static const struct brw_device_info 
brw_device_info_skl_gt3 = {
 
 static const struct brw_device_info brw_device_info_skl_gt4 = {
GEN9_FEATURES, .gt = 4,
+   /* From the docs:
+*   URB is limited to 1008KB due to programming restrictions. This is not a
+*   restriction of the L3 implementation, but of the FF and other clients.
+*   Therefore, in a GT4 implementation it is possible for the programmed
+*   allocation of the L3 data array to provide 3*384KB=1152MB [sic] for 
URB,
+*   but only 1008KB of this will be used.
+*/
+   .urb.size = 1008 / 3,
 };
 
 static const struct brw_device_info brw_device_info_bxt = {
-- 
2.6.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965/skl/gt4: Fix URB programming restriction.

2015-11-06 Thread Jordan Justen
On 2015-11-06 18:12:27, Ben Widawsky wrote:
> The comment in the code details the restriction. Thanks to Ken for having a 
> very
> helpful conversation with me, and spotting the blurb in the link I sent him 
> :P.
> 
> There are still stability problems for me on GT4, but this definitely helps 
> with
> some of the failures.

The line wrapping seems a bit too long.

Reviewed-by: Jordan Justen 

> 
> Cc: Kenneth Graunke 
> Cc: Jordan Justen 
> Cc: mesa-sta...@lists.freedesktop.org (if the original gt4 patch goes to 
> stable)
> ---
> 
> Sarah, you should check this on KBL.
> Cc: Sarah Sharp 
> ---
>  src/mesa/drivers/dri/i965/brw_device_info.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_device_info.c 
> b/src/mesa/drivers/dri/i965/brw_device_info.c
> index 2ebc084..6117fd3 100644
> --- a/src/mesa/drivers/dri/i965/brw_device_info.c
> +++ b/src/mesa/drivers/dri/i965/brw_device_info.c
> @@ -337,6 +337,14 @@ static const struct brw_device_info 
> brw_device_info_skl_gt3 = {
>  
>  static const struct brw_device_info brw_device_info_skl_gt4 = {
> GEN9_FEATURES, .gt = 4,
> +   /* From the docs:
> +*   URB is limited to 1008KB due to programming restrictions. This is 
> not a
> +*   restriction of the L3 implementation, but of the FF and other 
> clients.
> +*   Therefore, in a GT4 implementation it is possible for the programmed
> +*   allocation of the L3 data array to provide 3*384KB=1152MB [sic] for 
> URB,
> +*   but only 1008KB of this will be used.
> +*/
> +   .urb.size = 1008 / 3,
>  };
>  
>  static const struct brw_device_info brw_device_info_bxt = {
> -- 
> 2.6.2
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965/skl/gt4: Fix URB programming restriction.

2015-11-06 Thread Kenneth Graunke
On Friday, November 06, 2015 06:12:27 PM Ben Widawsky wrote:
> The comment in the code details the restriction. Thanks to Ken for having a 
> very
> helpful conversation with me, and spotting the blurb in the link I sent him 
> :P.
> 
> There are still stability problems for me on GT4, but this definitely helps 
> with
> some of the failures.
> 
> Cc: Kenneth Graunke 
> Cc: Jordan Justen 
> Cc: mesa-sta...@lists.freedesktop.org (if the original gt4 patch goes to 
> stable)
> ---
> 
> Sarah, you should check this on KBL.
> Cc: Sarah Sharp 
> ---
>  src/mesa/drivers/dri/i965/brw_device_info.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_device_info.c 
> b/src/mesa/drivers/dri/i965/brw_device_info.c
> index 2ebc084..6117fd3 100644
> --- a/src/mesa/drivers/dri/i965/brw_device_info.c
> +++ b/src/mesa/drivers/dri/i965/brw_device_info.c
> @@ -337,6 +337,14 @@ static const struct brw_device_info 
> brw_device_info_skl_gt3 = {
>  
>  static const struct brw_device_info brw_device_info_skl_gt4 = {
> GEN9_FEATURES, .gt = 4,
> +   /* From the docs:
> +*   URB is limited to 1008KB due to programming restrictions. This is 
> not a
> +*   restriction of the L3 implementation, but of the FF and other 
> clients.
> +*   Therefore, in a GT4 implementation it is possible for the programmed
> +*   allocation of the L3 data array to provide 3*384KB=1152MB [sic] for 
> URB,
> +*   but only 1008KB of this will be used.
> +*/

We should at least say which page/section this comes from (the section
name exists in the Broadwell PRM, so it's not particularly secret).

Please put the text in quotes too.  My suggested formatting:

   /* From the "L3 Allocation and Programming" documentation:
*
* "URB is limited to 1008KB due to programming restrictions.  This
*  is not a restriction of the L3 implementation, but of the FF and
*  other clients.  Therefore, in a GT4 implementation it is
*  possible for the programmed allocation of the L3 data array to
*  provide 3*384KB=1152MB [sic] for URB, but only 1008KB of this
*  will be used."
*/

Regardless,
Reviewed-by: Kenneth Graunke 

> +   .urb.size = 1008 / 3,
>  };
>  
>  static const struct brw_device_info brw_device_info_bxt = {
> 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev