Re: [Mesa-dev] [PATCH 6/6] anv: add dispatch macro to find right function for given generation

2016-10-17 Thread Jason Ekstrand
As a side-note, I pushed code on Friday that gets rid of two of these (the
create_pipeline calls).  I think the best way to solve
emit_state_base_address is by moving a bit of code into genX_cmd_buffer.c.
null surface states probably need to be moved into ISL.

In other words, while it's a nice cleanup, I think the correct answer for
almost all cases is to just stop doing the switches.

On Mon, Oct 17, 2016 at 8:51 AM, Kenneth Graunke 
wrote:

> On Wednesday, October 12, 2016 11:28:04 PM PDT Lionel Landwerlin wrote:
> > Signed-off-by: Lionel Landwerlin 
> > ---
> >  src/intel/vulkan/anv_cmd_buffer.c | 33 ++
> ---
> >  src/intel/vulkan/anv_device.c | 19 +--
> >  src/intel/vulkan/anv_pipeline.c   | 30 --
> >  src/intel/vulkan/anv_private.h| 23 +++
> >  4 files changed, 34 insertions(+), 71 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_cmd_buffer.c
> b/src/intel/vulkan/anv_cmd_buffer.c
> > index 5bcd5e0..b051489 100644
> > --- a/src/intel/vulkan/anv_cmd_buffer.c
> > +++ b/src/intel/vulkan/anv_cmd_buffer.c
> > @@ -356,19 +356,9 @@ VkResult anv_ResetCommandBuffer(
> >  void
> >  anv_cmd_buffer_emit_state_base_address(struct anv_cmd_buffer
> *cmd_buffer)
> >  {
> > -   switch (cmd_buffer->device->info.gen) {
> > -   case 7:
> > -  if (cmd_buffer->device->info.is_haswell)
> > - return gen75_cmd_buffer_emit_state_base_address(cmd_buffer);
> > -  else
> > - return gen7_cmd_buffer_emit_state_base_address(cmd_buffer);
> > -   case 8:
> > -  return gen8_cmd_buffer_emit_state_base_address(cmd_buffer);
> > -   case 9:
> > -  return gen9_cmd_buffer_emit_state_base_address(cmd_buffer);
> > -   default:
> > -  unreachable("unsupported gen\n");
> > -   }
> > +   ANV_GEN_DISPATCH(cmd_buffer->device,
> > +cmd_buffer_emit_state_base_address,
> > +cmd_buffer);
> >  }
> >
> >  VkResult anv_BeginCommandBuffer(
> > @@ -714,20 +704,9 @@ static struct anv_state
> >  anv_cmd_buffer_alloc_null_surface_state(struct anv_cmd_buffer
> *cmd_buffer,
> >  struct anv_framebuffer *fb)
> >  {
> > -   switch (cmd_buffer->device->info.gen) {
> > -   case 7:
> > -  if (cmd_buffer->device->info.is_haswell) {
> > - return gen75_cmd_buffer_alloc_null_surface_state(cmd_buffer,
> fb);
> > -  } else {
> > - return gen7_cmd_buffer_alloc_null_surface_state(cmd_buffer,
> fb);
> > -  }
> > -   case 8:
> > -  return gen8_cmd_buffer_alloc_null_surface_state(cmd_buffer, fb);
> > -   case 9:
> > -  return gen9_cmd_buffer_alloc_null_surface_state(cmd_buffer, fb);
> > -   default:
> > -  unreachable("Invalid hardware generation");
> > -   }
> > +   return ANV_GEN_DISPATCH(cmd_buffer->device,
> > +   cmd_buffer_alloc_null_surface_state,
> > +   cmd_buffer, fb);
> >  }
> >
> >  VkResult
> > diff --git a/src/intel/vulkan/anv_device.c
> b/src/intel/vulkan/anv_device.c
> > index 24f7227..6dfdfdb 100644
> > --- a/src/intel/vulkan/anv_device.c
> > +++ b/src/intel/vulkan/anv_device.c
> > @@ -921,24 +921,7 @@ VkResult anv_CreateDevice(
> >
> > anv_queue_init(device, &device->queue);
> >
> > -   switch (device->info.gen) {
> > -   case 7:
> > -  if (!device->info.is_haswell)
> > - result = gen7_init_device_state(device);
> > -  else
> > - result = gen75_init_device_state(device);
> > -  break;
> > -   case 8:
> > -  result = gen8_init_device_state(device);
> > -  break;
> > -   case 9:
> > -  result = gen9_init_device_state(device);
> > -  break;
> > -   default:
> > -  /* Shouldn't get here as we don't create physical devices for any
> other
> > -   * gens. */
> > -  unreachable("unhandled gen");
> > -   }
> > +   result = ANV_GEN_DISPATCH(device, init_device_state, device);
> > if (result != VK_SUCCESS)
> >goto fail_fd;
> >
> > diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_
> pipeline.c
> > index 6b393a6..f9f1cbf 100644
> > --- a/src/intel/vulkan/anv_pipeline.c
> > +++ b/src/intel/vulkan/anv_pipeline.c
> > @@ -1182,19 +1182,8 @@ anv_graphics_pipeline_create(
> > ANV_FROM_HANDLE(anv_device, device, _device);
> > ANV_FROM_HANDLE(anv_pipeline_cache, cache, _cache);
> >
> > -   switch (device->info.gen) {
> > -   case 7:
> > -  if (device->info.is_haswell)
> > - return gen75_graphics_pipeline_create(_device, cache,
> pCreateInfo, extra, pAllocator, pPipeline);
> > -  else
> > - return gen7_graphics_pipeline_create(_device, cache,
> pCreateInfo, extra, pAllocator, pPipeline);
> > -   case 8:
> > -  return gen8_graphics_pipeline_create(_device, cache,
> pCreateInfo, extra, pAllocator, pPipeline);
> > -   case 9:
> > -  return gen9_graphics_pipeline_create(_device, cache,
> pCreateInfo, extra, pAllocator, pPipeline);
> > -   default:
> > -

Re: [Mesa-dev] [PATCH 6/6] anv: add dispatch macro to find right function for given generation

2016-10-17 Thread Kenneth Graunke
On Wednesday, October 12, 2016 11:28:04 PM PDT Lionel Landwerlin wrote:
> Signed-off-by: Lionel Landwerlin 
> ---
>  src/intel/vulkan/anv_cmd_buffer.c | 33 ++---
>  src/intel/vulkan/anv_device.c | 19 +--
>  src/intel/vulkan/anv_pipeline.c   | 30 --
>  src/intel/vulkan/anv_private.h| 23 +++
>  4 files changed, 34 insertions(+), 71 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_cmd_buffer.c 
> b/src/intel/vulkan/anv_cmd_buffer.c
> index 5bcd5e0..b051489 100644
> --- a/src/intel/vulkan/anv_cmd_buffer.c
> +++ b/src/intel/vulkan/anv_cmd_buffer.c
> @@ -356,19 +356,9 @@ VkResult anv_ResetCommandBuffer(
>  void
>  anv_cmd_buffer_emit_state_base_address(struct anv_cmd_buffer *cmd_buffer)
>  {
> -   switch (cmd_buffer->device->info.gen) {
> -   case 7:
> -  if (cmd_buffer->device->info.is_haswell)
> - return gen75_cmd_buffer_emit_state_base_address(cmd_buffer);
> -  else
> - return gen7_cmd_buffer_emit_state_base_address(cmd_buffer);
> -   case 8:
> -  return gen8_cmd_buffer_emit_state_base_address(cmd_buffer);
> -   case 9:
> -  return gen9_cmd_buffer_emit_state_base_address(cmd_buffer);
> -   default:
> -  unreachable("unsupported gen\n");
> -   }
> +   ANV_GEN_DISPATCH(cmd_buffer->device,
> +cmd_buffer_emit_state_base_address,
> +cmd_buffer);
>  }
>  
>  VkResult anv_BeginCommandBuffer(
> @@ -714,20 +704,9 @@ static struct anv_state
>  anv_cmd_buffer_alloc_null_surface_state(struct anv_cmd_buffer *cmd_buffer,
>  struct anv_framebuffer *fb)
>  {
> -   switch (cmd_buffer->device->info.gen) {
> -   case 7:
> -  if (cmd_buffer->device->info.is_haswell) {
> - return gen75_cmd_buffer_alloc_null_surface_state(cmd_buffer, fb);
> -  } else {
> - return gen7_cmd_buffer_alloc_null_surface_state(cmd_buffer, fb);
> -  }
> -   case 8:
> -  return gen8_cmd_buffer_alloc_null_surface_state(cmd_buffer, fb);
> -   case 9:
> -  return gen9_cmd_buffer_alloc_null_surface_state(cmd_buffer, fb);
> -   default:
> -  unreachable("Invalid hardware generation");
> -   }
> +   return ANV_GEN_DISPATCH(cmd_buffer->device,
> +   cmd_buffer_alloc_null_surface_state,
> +   cmd_buffer, fb);
>  }
>  
>  VkResult
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index 24f7227..6dfdfdb 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -921,24 +921,7 @@ VkResult anv_CreateDevice(
>  
> anv_queue_init(device, &device->queue);
>  
> -   switch (device->info.gen) {
> -   case 7:
> -  if (!device->info.is_haswell)
> - result = gen7_init_device_state(device);
> -  else
> - result = gen75_init_device_state(device);
> -  break;
> -   case 8:
> -  result = gen8_init_device_state(device);
> -  break;
> -   case 9:
> -  result = gen9_init_device_state(device);
> -  break;
> -   default:
> -  /* Shouldn't get here as we don't create physical devices for any other
> -   * gens. */
> -  unreachable("unhandled gen");
> -   }
> +   result = ANV_GEN_DISPATCH(device, init_device_state, device);
> if (result != VK_SUCCESS)
>goto fail_fd;
>  
> diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c
> index 6b393a6..f9f1cbf 100644
> --- a/src/intel/vulkan/anv_pipeline.c
> +++ b/src/intel/vulkan/anv_pipeline.c
> @@ -1182,19 +1182,8 @@ anv_graphics_pipeline_create(
> ANV_FROM_HANDLE(anv_device, device, _device);
> ANV_FROM_HANDLE(anv_pipeline_cache, cache, _cache);
>  
> -   switch (device->info.gen) {
> -   case 7:
> -  if (device->info.is_haswell)
> - return gen75_graphics_pipeline_create(_device, cache, pCreateInfo, 
> extra, pAllocator, pPipeline);
> -  else
> - return gen7_graphics_pipeline_create(_device, cache, pCreateInfo, 
> extra, pAllocator, pPipeline);
> -   case 8:
> -  return gen8_graphics_pipeline_create(_device, cache, pCreateInfo, 
> extra, pAllocator, pPipeline);
> -   case 9:
> -  return gen9_graphics_pipeline_create(_device, cache, pCreateInfo, 
> extra, pAllocator, pPipeline);
> -   default:
> -  unreachable("unsupported gen\n");
> -   }
> +   return ANV_GEN_DISPATCH(device, graphics_pipeline_create,
> +   _device, cache, pCreateInfo, extra, pAllocator, 
> pPipeline);
>  }
>  
>  VkResult anv_CreateGraphicsPipelines(
> @@ -1235,19 +1224,8 @@ static VkResult anv_compute_pipeline_create(
> ANV_FROM_HANDLE(anv_device, device, _device);
> ANV_FROM_HANDLE(anv_pipeline_cache, cache, _cache);
>  
> -   switch (device->info.gen) {
> -   case 7:
> -  if (device->info.is_haswell)
> - return gen75_compute_pipeline_create(_device, cache, pCreateInfo, 
> pAllocator, pPipeline);
> -  else
> - return gen7_