[PATCH 1/2] drm/ttm: remove unused placement flags
Christian König
christian.koenig at amd.com
Fri Sep 9 14:17:33 UTC 2016
Am 09.09.2016 um 15:41 schrieb Deucher, Alexander:
>> -----Original Message-----
>> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
>> Of Christian König
>> Sent: Friday, September 09, 2016 7:24 AM
>> To: Zhang, Hawking; Koenig, Christian; Cui, Flora; Zhou, David(ChunMing);
>> amd-gfx at lists.freedesktop.org
>> Cc: dri-devel at lists.freedesktop.org
>> Subject: Re: [PATCH 1/2] drm/ttm: remove unused placement flags
>>
>> Hi Hawking,
>>
>>> Removing the flag will make ttm_mem_type_from_place skip counting the
>> corresponding placement and thus have impact on mem region create and
>> bo movement.
>> And that is exactly the reason why I want to remove the unused flags.
>>
>>> There is no guarantee that amdgpu would never introduce new memory
>> domain in future.
>> Irrelevant, if any driver wants to use additional domains it should add
>> them when they are used.
>>
>> BTW: Why would we want to add another TTM domain? I really don't see any
>> need for that.
> We need it for the hybrid driver in the near feature and the open driver may use it in the future depending on the use cases. Removing it just makes our lives more difficult for supporting dkms and distro integration for very minimal gain.
Yeah, agree. The problem is I already did so without knowing that.
I will send out a V2 only removing the stuff we explicitly don't need.
Hopefully nobody will notice and question further,
Christian.
>
> Alex
>
>>> Then how about keep these flags?
>> Actually we used to have automated scanners which complain about unused
>> code. I'm wondering why they don't detected that earlier.
>>
>> Anyway any code which isn't used in a while should be removed.
>>
>> Regards,
>> Christian.
>>
>> Am 09.09.2016 um 12:35 schrieb Zhang, Hawking:
>>> Hi Chris,
>>>
>>> Removing the flag will make ttm_mem_type_from_place skip counting the
>> corresponding placement and thus have impact on mem region create and
>> bo movement. There is no guarantee that amdgpu would never introduce
>> new memory domain in future. In such case, I'd like to vote for keeping
>> these flags instead of adding them back when amdgpu need to add new
>> memory domain.
>>> As you mentioned that it just a two liner. And there is actually no
>> functionality break with these flags. Then how about keep these flags?
>>> Regards,
>>> Hawking
>>>
>>> -----Original Message-----
>>> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
>> Of Christian K?nig
>>> Sent: Friday, September 09, 2016 17:07
>>> To: Cui, Flora <Flora.Cui at amd.com>; Zhou, David(ChunMing)
>> <David1.Zhou at amd.com>
>>> Cc: amd-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org
>>> Subject: Re: [PATCH 1/2] drm/ttm: remove unused placement flags
>>>
>>> In this case please just add them back in your tree. That should be a two
>> liner.
>>> For upstream it certainly doesn't make sense to keep them.
>>>
>>> Regards,
>>> Christian.
>>>
>>> Am 09.09.2016 um 09:01 schrieb Flora Cui:
>>>> yes. please don't do this, I need them.
>>>>
>>>> On Fri, Sep 09, 2016 at 02:41:16PM +0800, zhoucm1 wrote:
>>>>> On 2016年09月08日 21:41, Christian König wrote:
>>>>>> From: Christian König <christian.koenig at amd.com>
>>>>>>
>>>>>> Either never used or not used in quite a while.
>>>>> No, I remember Flora's Direct GMA is using them like GDS use PRIV0-2.
>>>>> And you cannot make sure there isn't no one using them in other
>>>>> closed projects, right?
>>>>> If you removed now, that obviously will break her implementation and
>>>>> brings her many troubles.
>>>>>
>>>>>
>>>>> Regards,
>>>>> David Zhou
>>>>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>>>>> ---
>>>>>> drivers/gpu/drm/ttm/ttm_bo.c | 2 +-
>>>>>> include/drm/ttm/ttm_placement.h | 19 -------------------
>>>>>> 2 files changed, 1 insertion(+), 20 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
>>>>>> b/drivers/gpu/drm/ttm/ttm_bo.c index bc37f02..4d2e8f2 100644
>>>>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>>>>> @@ -59,7 +59,7 @@ static inline int ttm_mem_type_from_place(const
>> struct ttm_place *place,
>>>>>> {
>>>>>> int i;
>>>>>> - for (i = 0; i <= TTM_PL_PRIV5; i++)
>>>>>> + for (i = 0; i <= TTM_PL_PRIV2; i++)
>>>>>> if (place->flags & (1 << i)) {
>>>>>> *mem_type = i;
>>>>>> return 0;
>>>>>> diff --git a/include/drm/ttm/ttm_placement.h
>>>>>> b/include/drm/ttm/ttm_placement.h index 8ed44f9..20219d9 100644
>>>>>> --- a/include/drm/ttm/ttm_placement.h
>>>>>> +++ b/include/drm/ttm/ttm_placement.h
>>>>>> @@ -40,10 +40,6 @@
>>>>>> #define TTM_PL_PRIV0 3
>>>>>> #define TTM_PL_PRIV1 4
>>>>>> #define TTM_PL_PRIV2 5
>>>>>> -#define TTM_PL_PRIV3 6
>>>>>> -#define TTM_PL_PRIV4 7
>>>>>> -#define TTM_PL_PRIV5 8
>>>>>> -#define TTM_PL_SWAPPED 15
>>>>>> #define TTM_PL_FLAG_SYSTEM (1 << TTM_PL_SYSTEM)
>>>>>> #define TTM_PL_FLAG_TT (1 << TTM_PL_TT)
>>>>>> @@ -51,10 +47,6 @@
>>>>>> #define TTM_PL_FLAG_PRIV0 (1 << TTM_PL_PRIV0)
>>>>>> #define TTM_PL_FLAG_PRIV1 (1 << TTM_PL_PRIV1)
>>>>>> #define TTM_PL_FLAG_PRIV2 (1 << TTM_PL_PRIV2)
>>>>>> -#define TTM_PL_FLAG_PRIV3 (1 << TTM_PL_PRIV3)
>>>>>> -#define TTM_PL_FLAG_PRIV4 (1 << TTM_PL_PRIV4)
>>>>>> -#define TTM_PL_FLAG_PRIV5 (1 << TTM_PL_PRIV5)
>>>>>> -#define TTM_PL_FLAG_SWAPPED (1 << TTM_PL_SWAPPED)
>>>>>> #define TTM_PL_MASK_MEM 0x0000FFFF
>>>>>> /*
>>>>>> @@ -72,7 +64,6 @@
>>>>>> #define TTM_PL_FLAG_CACHED (1 << 16)
>>>>>> #define TTM_PL_FLAG_UNCACHED (1 << 17)
>>>>>> #define TTM_PL_FLAG_WC (1 << 18)
>>>>>> -#define TTM_PL_FLAG_SHARED (1 << 20)
>>>>>> #define TTM_PL_FLAG_NO_EVICT (1 << 21)
>>>>>> #define TTM_PL_FLAG_TOPDOWN (1 << 22)
>>>>>> @@ -82,14 +73,4 @@
>>>>>> #define TTM_PL_MASK_MEMTYPE (TTM_PL_MASK_MEM |
>> TTM_PL_MASK_CACHING)
>>>>>> -/*
>>>>>> - * Access flags to be used for CPU- and GPU- mappings.
>>>>>> - * The idea is that the TTM synchronization mechanism will
>>>>>> - * allow concurrent READ access and exclusive write access.
>>>>>> - * Currently GPU- and CPU accesses are exclusive.
>>>>>> - */
>>>>>> -
>>>>>> -#define TTM_ACCESS_READ (1 << 0)
>>>>>> -#define TTM_ACCESS_WRITE (1 << 1)
>>>>>> -
>>>>>> #endif
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx at lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the amd-gfx
mailing list