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
