[Mesa-dev] [PATCH 2/9] anv: Set the values for the VkPhysicalDeviceSampleLocationsPropertiesEXT

[Eleni Maria Stea]

On Mon, 11 Mar 2019 11:39:58 -0700
Sagar Ghuge <sghuge at intel.com> wrote:

> On Mon, 2019-03-11 at 17:04 +0200, Eleni Maria Stea wrote:
> > The VkPhysicalDeviceSampleLocationPropertiesEXT struct is filled
> > with implementation dependent values and according to the table
> > from the Vulkan Specification section [36.1. Limit Requirements]:
> > 
> > pname | max | min
> > pname:sampleLocationSampleCounts   |-
> > |ename:VK_SAMPLE_COU NT_4_BIT
> > pname:maxSampleLocationGridSize    |-            |(1, 1)
> > pname:sampleLocationCoordinateRange|(0.0, 0.9375)|(0.0, 0.9375)
> > pname:sampleLocationSubPixelBits   |-            |4
> > pname:variableSampleLocations      | false       |implementation
> > dependent
> > 
> > The hardware only supports setting the same sample location for all
> > the
> > pixels, so we only support 1x1 grids.
> > 
> > Also, variableSampleLocations is set to false because we don't
> > support the
> > feature.
> > ---
> >  src/intel/vulkan/anv_device.c  | 21 +++++++++++++++++++++
> >  src/intel/vulkan/anv_private.h |  3 +++
> >  2 files changed, 24 insertions(+)
> > 
> > diff --git a/src/intel/vulkan/anv_device.c
> > b/src/intel/vulkan/anv_device.c
> > index 729cceb3e32..1e183b7f4ad 100644
> > --- a/src/intel/vulkan/anv_device.c
> > +++ b/src/intel/vulkan/anv_device.c
> > @@ -1401,6 +1401,27 @@ void anv_GetPhysicalDeviceProperties2(
> >           break;
> >        }
> >  
> > +      case
> > VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SAMPLE_LOCATIONS_PROPERTIES_EXT: {
> > +         VkPhysicalDeviceSampleLocationsPropertiesEXT *props =
> > +            (VkPhysicalDeviceSampleLocationsPropertiesEXT *)ext;
> > +         props->sampleLocationSampleCounts = ISL_SAMPLE_COUNT_2_BIT
> > |
> > +                                             ISL_SAMPLE_COUNT_4_BIT
> > |
> > +
> > ISL_SAMPLE_COUNT_8_BIT;
> > +         if (pdevice->info.gen >= 9)
> > +            props->sampleLocationSampleCounts |=
> > ISL_SAMPLE_COUNT_16_BIT;  
> 
> Hi Eleni,
> 
> Thanks for the series.
> 
> "isl_device_get_sample_counts" method figure out values according to
> platform so maybe we can make use of it and ignore
> ISL_SAMPLE_COUNT_1_BIT. So that we don't have to take care of values
> according to platform here. 
> 
> I am not sure about this, so it might be a good idea to consult with
> Jason/Lionel once. :)

I think that not only you are right here, but on top of that we
shouldn't ignore the ISL_SAMPLE_COUNT_1_BIT, as we can still write one
user defined location when only 1 sample per pixel is used (at least
MULTISAMPLE and SAMPLE_PATTERN commands allow us to do so). So, I've
made the change, thank you. :)

> 
> with or without the fix, this patch is:
> 
> Reviewed-by: Sagar Ghuge <sagar.ghuge at intel.com>
> 

Thanks for the review!
Eleni