[Mesa-dev] [RFC PATCH] gallium: add interface for EQAA

Tue May 1 15:36:58 UTC 2018

Am 01.05.2018 um 17:18 schrieb Nicolai Hähnle:
> On 01.05.2018 16:48, Roland Scheidegger wrote:
>>> -**nr_samples** the nr of msaa samples. 0 (or 1) specifies a resource
>>> -which isn't multisampled.
>>> +**nr_samples**: For Z/S, this is the number of samples. For color,
>>> if EQAA
>>> +is unsupported, this is the number of both coverage samples and
>>> color samples.
>>> +If EQAA is supported, this is the number of coverage samples. 0 and 1
>>> +specify a resource which isn't multisampled.
>> I think you should keep nr_samples alone, and re-change the meaning to
>> actually be "real" samples with all associated data (which is what
>> everybody expects with msaa).
>>
>>> +
>>> +**nr_color_samples**: This is the number of color samples for EQAA,
>>> while
>>> +``nr_samples`` is the number of coverage samples. If the format is Z/S,
>>> +``nr_color_samples`` is ignored. Constraints:
>>> +* ``nr_color_samples`` must not be greater than ``nr_samples``.
>>> +* If ``nr_color_samples`` is equal to ``nr_samples``, it is called
>>> MSAA.
>>> +* If ``nr_color_samples`` is less than ``nr_samples``, it is called
>>> EQAA.
>>> +* If ``nr_color_samples`` is equal to 1, the behavior of the resolve
>>> blit is
>>> +driver-dependent.
>>>   
>> Hence instead use something like "total_samples" or "coverage samples"
>> here (albeit the latter could be a bit confusing, unclear if it means
>> all samples or just the extra ones).
>> If you'd use something like "number of additional coverage samples"
>> instead, then you could even leave all state trackers alone, since 0
>> would automatically mean ordinary msaa without crazy stuff.
> 
> Having the new variable be about coverage samples makes sense.
> 
> I'm not convinced that having it be the number of *additional* samples
> is so convenient. At least in our hardware, there tend to be fields that
> encode the log of total samples of each type, so having a total number
> of coverage samples seems more convenient. That said, it's a bit
> bike-shedding.
Well you can easily just add "normal" samples and additional samples
before doing the log.
That said, I suppose you're right, since the adjustment needed to state
trackers for encoding the new field (with ordinary msaa) is trivial
(just always set to the same value) it's probably not worth worrying about.

Roland


> 
> Cheers,
> Nicolai
> 
> 
>> And fwiw I'd stick to the CSAA name - coverage at least has some
>> meaning, whereas "enhanced" (which is what the "E" in EQAA is for
>> according to some old articles) really is just completely meaningless
>> marketing term. There really isn't any difference, ok the ability to use
>> different amount of samples per rt might be something, but you can't
>> expose this anyway (AMD didn't come up with an extension for this since
>> the 7+ years it's supported (since NI), so I have some doubts of its
>> usefulness - besides you could have another cap which then would say if
>> that's valid or not if you really come up with an extension specifically
>> for it.)
>> Unless there's some non-technical reasons you can't use the term CSAA.
>>
>> Roland
>>
>>
>>
>>>   **usage** one of the :ref:`PIPE_USAGE` flags.
>>>     **bind** bitmask of the :ref:`PIPE_BIND` flags.
>>>     **flags** bitmask of PIPE_RESOURCE_FLAG flags.
>>>         resource_changed
>>> diff --git a/src/gallium/include/pipe/p_defines.h
>>> b/src/gallium/include/pipe/p_defines.h
>>> index c4ae0532060..97e1a3a3d42 100644
>>> --- a/src/gallium/include/pipe/p_defines.h
>>> +++ b/src/gallium/include/pipe/p_defines.h
>>> @@ -783,20 +783,21 @@ enum pipe_cap
>>>      PIPE_CAP_ALLOW_MAPPED_BUFFERS_DURING_EXECUTION,
>>>      PIPE_CAP_POST_DEPTH_COVERAGE,
>>>      PIPE_CAP_BINDLESS_TEXTURE,
>>>      PIPE_CAP_NIR_SAMPLERS_AS_DEREF,
>>>      PIPE_CAP_QUERY_SO_OVERFLOW,
>>>      PIPE_CAP_MEMOBJ,
>>>      PIPE_CAP_LOAD_CONSTBUF,
>>>      PIPE_CAP_TGSI_ANY_REG_AS_ADDRESS,
>>>      PIPE_CAP_TILE_RASTER_ORDER,
>>>      PIPE_CAP_MAX_COMBINED_SHADER_OUTPUT_RESOURCES,
>>> +   PIPE_CAP_EQAA_COLOR_SAMPLE_SUPPORT_MASK,
>>>      PIPE_CAP_SIGNED_VERTEX_BUFFER_OFFSET,
>>>      PIPE_CAP_CONTEXT_PRIORITY_MASK,
>>>      PIPE_CAP_FENCE_SIGNAL,
>>>      PIPE_CAP_CONSTBUF0_FLAGS,
>>>      PIPE_CAP_PACKED_UNIFORMS,
>>>   };
>>>     /**
>>>    * Possible bits for PIPE_CAP_CONTEXT_PRIORITY_MASK param, which
>>> should
>>>    * return a bitmask of the supported priorities.  If the driver
>>> does not
>>> diff --git a/src/gallium/include/pipe/p_state.h
>>> b/src/gallium/include/pipe/p_state.h
>>> index 4dce399f848..4010b92e67b 100644
>>> --- a/src/gallium/include/pipe/p_state.h
>>> +++ b/src/gallium/include/pipe/p_state.h
>>> @@ -503,41 +503,42 @@ struct pipe_box
>>>      int16_t depth;
>>>   };
>>>       /**
>>>    * A memory object/resource such as a vertex buffer or texture.
>>>    */
>>>   struct pipe_resource
>>>   {
>>>      struct pipe_reference reference;
>>> -   struct pipe_screen *screen; /**< screen that this texture belongs
>>> to */
>>>        unsigned width0; /**< Used by both buffers and textures. */
>>>      uint16_t height0; /* Textures: The maximum
>>> height/depth/array_size is 16k. */
>>>      uint16_t depth0;
>>>      uint16_t array_size;
>>>        enum pipe_format format:16;         /**< PIPE_FORMAT_x */
>>>      enum pipe_texture_target target:8; /**< PIPE_TEXTURE_x */
>>>      unsigned last_level:8;    /**< Index of last mipmap level
>>> present/defined */
>>>      unsigned nr_samples:8;    /**< for multisampled surfaces, nr of
>>> samples */
>>> +   unsigned nr_color_samples:8; /**< Number of color samples for
>>> EQAA. */
>>>      unsigned usage:8;         /**< PIPE_USAGE_x (not a bitmask) */
>>>        unsigned bind;            /**< bitmask of PIPE_BIND_x */
>>>      unsigned flags;           /**< bitmask of PIPE_RESOURCE_FLAG_x */
>>>        /**
>>>       * For planar images, ie. YUV EGLImage external, etc, pointer to
>>> the
>>>       * next plane.
>>>       */
>>>      struct pipe_resource *next;
>>> +   struct pipe_screen *screen; /**< screen that this texture belongs
>>> to */
>>>   };
>>>       /**
>>>    * Transfer object.  For data transfer to/from a resource.
>>>    */
>>>   struct pipe_transfer
>>>   {
>>>      struct pipe_resource *resource; /**< resource to transfer
>>> to/from  */
>>>      unsigned level;                 /**< texture mipmap level */
>>>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=DwICaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=P7tRmPIbcPNV6bsdITIz7qV6orXfgw-Yo9QHQFgAUXk&s=8cLAadC0mf2y_aegN7OxKdcdOKlzcjX1Jv_F6PpqdBk&e=
>>
>>
> 
>