Re: [PATCH 1/8] drm/ttm: add default implementations for ttm_tt_(un)populate

[Michel Dänzer]

On 2018-02-23 08:39 AM, Christian König wrote:
> Am 22.02.2018 um 16:55 schrieb Michel Dänzer:
>> On 2018-02-22 12:15 PM, Christian König wrote:
>>> Use ttm_pool_populate/ttm_pool_unpopulate if the driver doesn't provide
>>> a function.
>>>
>>> Signed-off-by: Christian König 
>>> ---
>>>   drivers/gpu/drm/ttm/ttm_tt.c | 10 --
>>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
>>> index 9fd7115a013a..65bf4eac184b 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_tt.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
>>> @@ -410,7 +410,10 @@ int ttm_tt_populate(struct ttm_tt *ttm, struct
>>> ttm_operation_ctx *ctx)
>>>   if (ttm->state != tt_unpopulated)
>>>   return 0;
>>>   -    ret = ttm->bdev->driver->ttm_tt_populate(ttm, ctx);
>>> +    if (ttm->bdev->driver->ttm_tt_populate)
>>> +    ret = ttm->bdev->driver->ttm_tt_populate(ttm, ctx);
>>> +    else
>>> +    ret = ttm_pool_populate(ttm, ctx);
>>>   if (!ret)
>>>   ttm_tt_add_mapping(ttm);
>>>   return ret;
>>> @@ -436,5 +439,8 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
>>>   return;
>>>     ttm_tt_clear_mapping(ttm);
>>> -    ttm->bdev->driver->ttm_tt_unpopulate(ttm);
>>> +    if (ttm->bdev->driver->ttm_tt_unpopulate)
>>> +    ttm->bdev->driver->ttm_tt_unpopulate(ttm);
>>> +    else
>>> +    ttm_pool_unpopulate(ttm);
>>>   }
>>>
>> Instead of the if/else, have you considered setting
>> driver->ttm_tt_(un)populate = ttm_pool_(un)populate during
>> initialization if they're NULL?
> 
> Mhm, also an interesting possibility. But shouldn't the function
> pointers be const?

I guess that might be nice.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/8] drm/ttm: add default implementations for ttm_tt_(un)populate

[Christian König]

Am 22.02.2018 um 16:55 schrieb Michel Dänzer:

On 2018-02-22 12:15 PM, Christian König wrote:

Use ttm_pool_populate/ttm_pool_unpopulate if the driver doesn't provide
a function.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 9fd7115a013a..65bf4eac184b 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -410,7 +410,10 @@ int ttm_tt_populate(struct ttm_tt *ttm, struct 
ttm_operation_ctx *ctx)
if (ttm->state != tt_unpopulated)
return 0;
  
-	ret = ttm->bdev->driver->ttm_tt_populate(ttm, ctx);

+   if (ttm->bdev->driver->ttm_tt_populate)
+   ret = ttm->bdev->driver->ttm_tt_populate(ttm, ctx);
+   else
+   ret = ttm_pool_populate(ttm, ctx);
if (!ret)
ttm_tt_add_mapping(ttm);
return ret;
@@ -436,5 +439,8 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
return;
  
  	ttm_tt_clear_mapping(ttm);

-   ttm->bdev->driver->ttm_tt_unpopulate(ttm);
+   if (ttm->bdev->driver->ttm_tt_unpopulate)
+   ttm->bdev->driver->ttm_tt_unpopulate(ttm);
+   else
+   ttm_pool_unpopulate(ttm);
  }


Instead of the if/else, have you considered setting
driver->ttm_tt_(un)populate = ttm_pool_(un)populate during
initialization if they're NULL?


Mhm, also an interesting possibility. But shouldn't the function 
pointers be const?


Christian.




Either way, the series is

Reviewed-by: Michel Dänzer 




___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/8] drm/ttm: add default implementations for ttm_tt_(un)populate

[Michel Dänzer]

On 2018-02-22 12:15 PM, Christian König wrote:
> Use ttm_pool_populate/ttm_pool_unpopulate if the driver doesn't provide
> a function.
> 
> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/ttm/ttm_tt.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index 9fd7115a013a..65bf4eac184b 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -410,7 +410,10 @@ int ttm_tt_populate(struct ttm_tt *ttm, struct 
> ttm_operation_ctx *ctx)
>   if (ttm->state != tt_unpopulated)
>   return 0;
>  
> - ret = ttm->bdev->driver->ttm_tt_populate(ttm, ctx);
> + if (ttm->bdev->driver->ttm_tt_populate)
> + ret = ttm->bdev->driver->ttm_tt_populate(ttm, ctx);
> + else
> + ret = ttm_pool_populate(ttm, ctx);
>   if (!ret)
>   ttm_tt_add_mapping(ttm);
>   return ret;
> @@ -436,5 +439,8 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
>   return;
>  
>   ttm_tt_clear_mapping(ttm);
> - ttm->bdev->driver->ttm_tt_unpopulate(ttm);
> + if (ttm->bdev->driver->ttm_tt_unpopulate)
> + ttm->bdev->driver->ttm_tt_unpopulate(ttm);
> + else
> + ttm_pool_unpopulate(ttm);
>  }
> 

Instead of the if/else, have you considered setting
driver->ttm_tt_(un)populate = ttm_pool_(un)populate during
initialization if they're NULL?


Either way, the series is

Reviewed-by: Michel Dänzer 


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/8] drm/ttm: add default implementations for ttm_tt_(un)populate

[Christian König]

Use ttm_pool_populate/ttm_pool_unpopulate if the driver doesn't provide
a function.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_tt.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 9fd7115a013a..65bf4eac184b 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -410,7 +410,10 @@ int ttm_tt_populate(struct ttm_tt *ttm, struct 
ttm_operation_ctx *ctx)
if (ttm->state != tt_unpopulated)
return 0;
 
-   ret = ttm->bdev->driver->ttm_tt_populate(ttm, ctx);
+   if (ttm->bdev->driver->ttm_tt_populate)
+   ret = ttm->bdev->driver->ttm_tt_populate(ttm, ctx);
+   else
+   ret = ttm_pool_populate(ttm, ctx);
if (!ret)
ttm_tt_add_mapping(ttm);
return ret;
@@ -436,5 +439,8 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
return;
 
ttm_tt_clear_mapping(ttm);
-   ttm->bdev->driver->ttm_tt_unpopulate(ttm);
+   if (ttm->bdev->driver->ttm_tt_unpopulate)
+   ttm->bdev->driver->ttm_tt_unpopulate(ttm);
+   else
+   ttm_pool_unpopulate(ttm);
 }
-- 
2.14.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel