[Mesa-dev] [PATCH 4/7] i965: Make brw_cache_item structure private to brw_program_cache.c.

[Eduardo Lima Mitev]

On 01/18/2017 06:55 AM, Kenneth Graunke wrote:
> On Tuesday, January 17, 2017 8:51:48 AM PST Eduardo Lima Mitev wrote:
>> On 01/17/2017 08:14 AM, Kenneth Graunke wrote:
>>> struct brw_cache_item is an implementation detail of the program cache.
>>> We don't need to make those internals available to the entire driver.
>>>
>>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_context.h       | 19 -------------------
>>>  src/mesa/drivers/dri/i965/brw_program_cache.c | 18 ++++++++++++++++++
>>>  2 files changed, 18 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
>>> index d5e42516307..aa1499a1fe1 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_context.h
>>> +++ b/src/mesa/drivers/dri/i965/brw_context.h
>>> @@ -453,25 +453,6 @@ struct brw_ff_gs_prog_data {
>>>   */
>>>  #define SHADER_TIME_STRIDE 64
>>>  
>>> -struct brw_cache_item {
>>> -   /**
>>> -    * Effectively part of the key, cache_id identifies what kind of state
>>> -    * buffer is involved, and also which dirty flag should set.
>>> -    */
>>> -   enum brw_cache_id cache_id;
>>> -   /** 32-bit hash of the key data */
>>> -   GLuint hash;
>>> -   GLuint key_size;		/* for variable-sized keys */
>>> -   GLuint aux_size;
>>> -   const void *key;
>>> -
>>> -   uint32_t offset;
>>> -   uint32_t size;
>>> -
>>> -   struct brw_cache_item *next;
>>> -};
>>> -
>>> -
>>>  struct brw_cache {
>>>     struct brw_context *brw;
>>>  
>>> diff --git a/src/mesa/drivers/dri/i965/brw_program_cache.c b/src/mesa/drivers/dri/i965/brw_program_cache.c
>>> index 44d9994de01..4d249ba6f93 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_program_cache.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_program_cache.c
>>> @@ -55,6 +55,24 @@
>>>  
>>>  #define FILE_DEBUG_FLAG DEBUG_STATE
>>>  
>>> +struct brw_cache_item {
>>> +   /**
>>> +    * Effectively part of the key, cache_id identifies what kind of state
>>> +    * buffer is involved, and also which dirty flag should set.
>>> +    */
>>>
>>
>> Maybe a good opportunity to move the comment above outside, right on top
>> of "struct brw_cache_item", for consistency with other symbol comments here.
> 
> I don't understand, sorry...are you suggesting moving the "Effectively
> part of the key..." comment outside the structure?  It's a comment about
> the cache_id field in the struct, so shouldn't it stay with the field?
> 

Oh, true! Sorry, I thought it was a whole structure comment. Please ignore.

>>> +   enum brw_cache_id cache_id;
>>> +   /** 32-bit hash of the key data */
>>>
>>
>> /* 32-bit hash of the key data */ (single start instead)
> 
> /** blah */ is Doxygen single-line form for a field member annotation.
> We use that in a lot of places.
> 

Ok, but note that few lines below there is another field comment without
the double star. Not that I care much about that level of consistency,
but since we are at it, we could choose either style.

Eduardo

>> Reviewed-by: Eduardo Lima Mitev <elima at igalia.com>