[PATCH] drm/amdgpu/gfx: create a common bitmask function (v2)

axie axie at amd.com
Thu Jun 8 14:42:27 UTC 2017


Hi Christian,


Which macro do you want to use to replace this function?


The function is small so that we use a "static inline". I agree that 
large function should not use "static inline". Thanks for point that out.


Alex Bin Xie


On 2017-06-08 03:20 AM, Christian König wrote:
> Actually we should only use static inline if we have to, see "15) The 
> inline disease" in the coding style document, but for this case it 
> actually looks valid to me as well.
>
> But wasn't there a macro for this in the Linux kernel headers we 
> should use?
>
> Regards,
> Christian.
>
> Am 07.06.2017 um 00:36 schrieb axie:
>>
>> Thanks for the change. "static inline" may improve speed, though this 
>> patch is not a critical code path. Perhaps it will be true in future...
>>
>> Reviewed-by: Alex Xie <AlexBin.Xie at amd.com>
>>
>>
>> On 2017-06-06 06:19 PM, Alex Deucher wrote:
>>> The same function was duplicated in all the gfx IPs. Use
>>> a single implementation for all.
>>>
>>> v2: use static inline (Alex Xie)
>>>
>>> Suggested-by: Andres Rodriguez<andresx7 at gmail.com>
>>> Signed-off-by: Alex Deucher<alexander.deucher at amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 13 +++++++++++++
>>>   drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c   | 11 +++--------
>>>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   | 19 +++----------------
>>>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 11 +++--------
>>>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 11 +++--------
>>>   5 files changed, 25 insertions(+), 40 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>>> index e0204408..2d846ef 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>>> @@ -30,4 +30,17 @@ void amdgpu_gfx_scratch_free(struct amdgpu_device *adev, uint32_t reg);
>>>   void amdgpu_gfx_parse_disable_cu(unsigned *mask, unsigned max_se,
>>>   		unsigned max_sh);
>>>   
>>> +/**
>>> + * amdgpu_gfx_create_bitmask - create a bitmask
>>> + *
>>> + * @bit_width: length of the mask
>>> + *
>>> + * create a variable length bit mask.
>>> + * Returns the bitmask.
>>> + */
>>> +static inline u32 amdgpu_gfx_create_bitmask(u32 bit_width)
>>> +{
>>> +	return (u32)((1ULL << bit_width) - 1);
>>> +}
>>> +
>>>   #endif
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
>>> index f3e0edd..cfd3df6 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
>>> @@ -1116,11 +1116,6 @@ static void gfx_v6_0_select_se_sh(struct amdgpu_device *adev, u32 se_num,
>>>   	WREG32(mmGRBM_GFX_INDEX, data);
>>>   }
>>>   
>>> -static u32 gfx_v6_0_create_bitmask(u32 bit_width)
>>> -{
>>> -	return (u32)(((u64)1 << bit_width) - 1);
>>> -}
>>> -
>>>   static u32 gfx_v6_0_get_rb_active_bitmap(struct amdgpu_device *adev)
>>>   {
>>>   	u32 data, mask;
>>> @@ -1130,8 +1125,8 @@ static u32 gfx_v6_0_get_rb_active_bitmap(struct amdgpu_device *adev)
>>>   
>>>   	data = REG_GET_FIELD(data, GC_USER_RB_BACKEND_DISABLE, BACKEND_DISABLE);
>>>   
>>> -	mask = gfx_v6_0_create_bitmask(adev->gfx.config.max_backends_per_se/
>>> -					adev->gfx.config.max_sh_per_se);
>>> +	mask = amdgpu_gfx_create_bitmask(adev->gfx.config.max_backends_per_se/
>>> +					 adev->gfx.config.max_sh_per_se);
>>>   
>>>   	return ~data & mask;
>>>   }
>>> @@ -1333,7 +1328,7 @@ static u32 gfx_v6_0_get_cu_enabled(struct amdgpu_device *adev)
>>>   	data = RREG32(mmCC_GC_SHADER_ARRAY_CONFIG) |
>>>   		RREG32(mmGC_USER_SHADER_ARRAY_CONFIG);
>>>   
>>> -	mask = gfx_v6_0_create_bitmask(adev->gfx.config.max_cu_per_sh);
>>> +	mask = amdgpu_gfx_create_bitmask(adev->gfx.config.max_cu_per_sh);
>>>   	return ~REG_GET_FIELD(data, CC_GC_SHADER_ARRAY_CONFIG, INACTIVE_CUS) & mask;
>>>   }
>>>   
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>> index ae98611..4c04e9d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>> @@ -1608,19 +1608,6 @@ static void gfx_v7_0_select_se_sh(struct amdgpu_device *adev,
>>>   }
>>>   
>>>   /**
>>> - * gfx_v7_0_create_bitmask - create a bitmask
>>> - *
>>> - * @bit_width: length of the mask
>>> - *
>>> - * create a variable length bit mask (CIK).
>>> - * Returns the bitmask.
>>> - */
>>> -static u32 gfx_v7_0_create_bitmask(u32 bit_width)
>>> -{
>>> -	return (u32)((1ULL << bit_width) - 1);
>>> -}
>>> -
>>> -/**
>>>    * gfx_v7_0_get_rb_active_bitmap - computes the mask of enabled RBs
>>>    *
>>>    * @adev: amdgpu_device pointer
>>> @@ -1638,8 +1625,8 @@ static u32 gfx_v7_0_get_rb_active_bitmap(struct amdgpu_device *adev)
>>>   	data &= CC_RB_BACKEND_DISABLE__BACKEND_DISABLE_MASK;
>>>   	data >>= GC_USER_RB_BACKEND_DISABLE__BACKEND_DISABLE__SHIFT;
>>>   
>>> -	mask = gfx_v7_0_create_bitmask(adev->gfx.config.max_backends_per_se /
>>> -				       adev->gfx.config.max_sh_per_se);
>>> +	mask = amdgpu_gfx_create_bitmask(adev->gfx.config.max_backends_per_se /
>>> +					 adev->gfx.config.max_sh_per_se);
>>>   
>>>   	return (~data) & mask;
>>>   }
>>> @@ -4157,7 +4144,7 @@ static u32 gfx_v7_0_get_cu_active_bitmap(struct amdgpu_device *adev)
>>>   	data &= CC_GC_SHADER_ARRAY_CONFIG__INACTIVE_CUS_MASK;
>>>   	data >>= CC_GC_SHADER_ARRAY_CONFIG__INACTIVE_CUS__SHIFT;
>>>   
>>> -	mask = gfx_v7_0_create_bitmask(adev->gfx.config.max_cu_per_sh);
>>> +	mask = amdgpu_gfx_create_bitmask(adev->gfx.config.max_cu_per_sh);
>>>   
>>>   	return (~data) & mask;
>>>   }
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>> index afd7d65..ad2e0bb 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>> @@ -3635,11 +3635,6 @@ static void gfx_v8_0_select_se_sh(struct amdgpu_device *adev,
>>>   	WREG32(mmGRBM_GFX_INDEX, data);
>>>   }
>>>   
>>> -static u32 gfx_v8_0_create_bitmask(u32 bit_width)
>>> -{
>>> -	return (u32)((1ULL << bit_width) - 1);
>>> -}
>>> -
>>>   static u32 gfx_v8_0_get_rb_active_bitmap(struct amdgpu_device *adev)
>>>   {
>>>   	u32 data, mask;
>>> @@ -3649,8 +3644,8 @@ static u32 gfx_v8_0_get_rb_active_bitmap(struct amdgpu_device *adev)
>>>   
>>>   	data = REG_GET_FIELD(data, GC_USER_RB_BACKEND_DISABLE, BACKEND_DISABLE);
>>>   
>>> -	mask = gfx_v8_0_create_bitmask(adev->gfx.config.max_backends_per_se /
>>> -				       adev->gfx.config.max_sh_per_se);
>>> +	mask = amdgpu_gfx_create_bitmask(adev->gfx.config.max_backends_per_se /
>>> +					 adev->gfx.config.max_sh_per_se);
>>>   
>>>   	return (~data) & mask;
>>>   }
>>> @@ -7150,7 +7145,7 @@ static u32 gfx_v8_0_get_cu_active_bitmap(struct amdgpu_device *adev)
>>>   	data =  RREG32(mmCC_GC_SHADER_ARRAY_CONFIG) |
>>>   		RREG32(mmGC_USER_SHADER_ARRAY_CONFIG);
>>>   
>>> -	mask = gfx_v8_0_create_bitmask(adev->gfx.config.max_cu_per_sh);
>>> +	mask = amdgpu_gfx_create_bitmask(adev->gfx.config.max_cu_per_sh);
>>>   
>>>   	return ~REG_GET_FIELD(data, CC_GC_SHADER_ARRAY_CONFIG, INACTIVE_CUS) & mask;
>>>   }
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>> index 276dc06..cf15a350 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>> @@ -1698,11 +1698,6 @@ static void gfx_v9_0_select_se_sh(struct amdgpu_device *adev, u32 se_num, u32 sh
>>>   	WREG32_SOC15(GC, 0, mmGRBM_GFX_INDEX, data);
>>>   }
>>>   
>>> -static u32 gfx_v9_0_create_bitmask(u32 bit_width)
>>> -{
>>> -	return (u32)((1ULL << bit_width) - 1);
>>> -}
>>> -
>>>   static u32 gfx_v9_0_get_rb_active_bitmap(struct amdgpu_device *adev)
>>>   {
>>>   	u32 data, mask;
>>> @@ -1713,8 +1708,8 @@ static u32 gfx_v9_0_get_rb_active_bitmap(struct amdgpu_device *adev)
>>>   	data &= CC_RB_BACKEND_DISABLE__BACKEND_DISABLE_MASK;
>>>   	data >>= GC_USER_RB_BACKEND_DISABLE__BACKEND_DISABLE__SHIFT;
>>>   
>>> -	mask = gfx_v9_0_create_bitmask(adev->gfx.config.max_backends_per_se /
>>> -				       adev->gfx.config.max_sh_per_se);
>>> +	mask = amdgpu_gfx_create_bitmask(adev->gfx.config.max_backends_per_se /
>>> +					 adev->gfx.config.max_sh_per_se);
>>>   
>>>   	return (~data) & mask;
>>>   }
>>> @@ -4609,7 +4604,7 @@ static u32 gfx_v9_0_get_cu_active_bitmap(struct amdgpu_device *adev)
>>>   	data &= CC_GC_SHADER_ARRAY_CONFIG__INACTIVE_CUS_MASK;
>>>   	data >>= CC_GC_SHADER_ARRAY_CONFIG__INACTIVE_CUS__SHIFT;
>>>   
>>> -	mask = gfx_v9_0_create_bitmask(adev->gfx.config.max_cu_per_sh);
>>> +	mask = amdgpu_gfx_create_bitmask(adev->gfx.config.max_cu_per_sh);
>>>   
>>>   	return (~data) & mask;
>>>   }
>>
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20170608/9a14f023/attachment-0001.html>


More information about the amd-gfx mailing list