| Message ID | 20180222111540.43479-1-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <christian.koenig@amd.com> > --- > 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 <michel.daenzer@amd.com>
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 <christian.koenig@amd.com> >> --- >> 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 <michel.daenzer@amd.com> > >
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 <christian.koenig@amd.com> >>> --- >>> 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.
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); }
Use ttm_pool_populate/ttm_pool_unpopulate if the driver doesn't provide a function. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/ttm/ttm_tt.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)