[Mesa-dev] [PATCH] i965/skl: Remove early platform support

[Ben Widawsky]

On Tue, Aug 11, 2015 at 03:04:17PM +0100, Neil Roberts wrote:
> Ben Widawsky <ben at bwidawsk.net> writes:
> 
> > Either of you opposed to doing it as a follow-on patch? (This failure
> > you speak of was the primary goal for removing it). Initially, I tried
> > to remove it, but we do still seem to use this logic pre-HSW, and so
> > things got messier than I initially anticipated.
> 
> Doing a follow-on patch is fine by me. I'm not sure what you mean by the
> logic being used pre-HSW, but I guess I'll see when you post the patch
> :)
> 

I'm looking at this again. It looks to me like leaving this as it is works best.
Here is the rough idea of what you get if you remove it (I didn't actually give
this a second glance, I just made it compile).

diff --git a/src/mesa/drivers/dri/i965/brw_device_info.c b/src/mesa/drivers/dri/i965/brw_device_info.c
index 7ad3a2f..9d47553 100644
--- a/src/mesa/drivers/dri/i965/brw_device_info.c
+++ b/src/mesa/drivers/dri/i965/brw_device_info.c
@@ -172,3 +172,2 @@ static const struct brw_device_info brw_device_info_byt = {
    .is_haswell = true,           \
-   .supports_simd16_3src = true, \
    .has_resource_streamer = true
@@ -236,3 +235,2 @@ static const struct brw_device_info brw_device_info_hsw_gt3 = {
    .has_pln = true,                                 \
-   .supports_simd16_3src = true,                    \
    .max_vs_threads = 504,                           \
@@ -308,3 +306,2 @@ static const struct brw_device_info brw_device_info_chv = {
    .has_pln = true,                                 \
-   .supports_simd16_3src = true,                    \
    .max_vs_threads = 336,                           \
diff --git a/src/mesa/drivers/dri/i965/brw_device_info.h b/src/mesa/drivers/dri/i965/brw_device_info.h
index 2a73e93..5c976b1 100644
--- a/src/mesa/drivers/dri/i965/brw_device_info.h
+++ b/src/mesa/drivers/dri/i965/brw_device_info.h
@@ -47,3 +47,2 @@ struct brw_device_info
    bool has_surface_tile_offset;
-   bool supports_simd16_3src;
    bool has_resource_streamer;
diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
index c86ca04..45a3544 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
@@ -1609,3 +1609,5 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
         brw_set_default_access_mode(p, BRW_ALIGN_16);
-         if (dispatch_width == 16 && !devinfo->supports_simd16_3src) {
+         /* SNB and IVB only */
+         if (dispatch_width == 16 && devinfo->gen < 8 && !devinfo->is_haswell &&
+             !devinfo->is_baytrail) {
             brw_set_default_exec_size(p, BRW_EXECUTE_8);
@@ -1631,3 +1633,4 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
         brw_set_default_access_mode(p, BRW_ALIGN_16);
-         if (dispatch_width == 16 && !devinfo->supports_simd16_3src) {
+         if (dispatch_width == 16 && devinfo->gen < 8 && !devinfo->is_haswell &&
+             !devinfo->is_baytrail) {
             brw_set_default_exec_size(p, BRW_EXECUTE_8);
@@ -1770,3 +1773,4 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
          brw_set_default_access_mode(p, BRW_ALIGN_16);
-         if (dispatch_width == 16 && !devinfo->supports_simd16_3src) {
+         if (dispatch_width == 16 && devinfo->gen < 8 && !devinfo->is_haswell &&
+             !devinfo->is_baytrail) {
             brw_set_default_exec_size(p, BRW_EXECUTE_8);
@@ -1812,4 +1816,6 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
           */
+         /* haswell, or ivybridge */
          if (dispatch_width == 16 &&
-             (devinfo->is_haswell || !devinfo->supports_simd16_3src)) {
+             (devinfo->is_haswell ||
+              (devinfo->gen == 7 && !devinfo->is_baytrail))) {
             brw_set_default_exec_size(p, BRW_EXECUTE_8);




> > Neil, you said "if we go with this patch" - I haven't heard any
> > opposition, do you have any?
> 
> No, it seems like a good idea to me.
> 
> Reviewed-by: Neil Roberts <neil at linux.intel.com>
> 
> - Neil

Thanks.