| Message ID | CADnq5_NKMMkjDvtVdKStTZYTHPcDeazFYdqDui-4z5dgL7SFGQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 30 September 2015 at 01:41, Alex Deucher <alexdeucher@gmail.com> wrote: > On Tue, Sep 29, 2015 at 11:20 AM, Daniel Vetter <daniel@ffwll.ch> wrote: >> On Tue, Sep 29, 2015 at 02:39:49PM +0200, Christian König wrote: >>> On 29.09.2015 13:40, Daniel Vetter wrote: >>> >On Thu, Jul 09, 2015 at 12:21:06PM -0400, Alex Deucher wrote: >>> >>From: Chunming Zhou <david1.zhou@amd.com> >>> >> >>> >>This implements the cgs interface for allocating >>> >>GPU memory. >>> >> >>> >>Reviewed-by: Jammy Zhou <Jammy.Zhou@amd.com> >>> >I don't see that review anywhere on a m-l ... where is it? >>> >>> Jammy reviewed the stuff internally before we made it public, that's why you >>> can't find it. Can we get these reviews done publically? it's kinda hard to know how well someone reviewed things if we have no external copy. Like did Jammy a) read the patch, and slap Reviewed-by on it, or did he b) comment on some whitespace issues and slap R-b on it, or c) did he suggest a bunch of changes and those changes were made and a new version was produced and he r-b'ed that etc. >> The other stuff seems a lot more benign. For the irq abstraction >> specifically it might be worth looking at the irq_chip stuff linux core >> has, which is what's used to virtualize/abstract irq routing and handling. >> But for that stuff it's a balance thing really how much you reinvent >> wheels internally in the driver (e.g. i915 has it's own power_well stuff >> which is pretty much just powerdomains reinvented, with less features). >> > > I think that's one of the hardest things in the kernel: finding out if > a solution already exists or not. We implemented our own version of > mfd for our ACP audio block driver. Upon upsteaming we were alerted > to mfd's existence and we converted the driver to use mfd. At the end > of the day it was a lot of work for minimal gain, at least from a > functionality perspective. I wish we had known about it sooner. I'll > take a look at the irq_chip stuff. Thanks for the heads up! You say for minimal gain, but this is pretty much going to keep happening to you with the development model you have chosen, get used to rewriting things you consider finished and reviewed. I've said it before so I'll use this to reiterate, your patches are only starting the process when you post them, all the internal stuff you do is nice and all but it could all be done externally if you guys weren't so stuck on internal IP review. Otherwise you should be taking into account that this overhead will continue to exist in all your development, and adjust schedules to suit. Dave.
On Tue, Sep 29, 2015 at 4:10 PM, Dave Airlie <airlied@gmail.com> wrote: > On 30 September 2015 at 01:41, Alex Deucher <alexdeucher@gmail.com> wrote: >> On Tue, Sep 29, 2015 at 11:20 AM, Daniel Vetter <daniel@ffwll.ch> wrote: >>> On Tue, Sep 29, 2015 at 02:39:49PM +0200, Christian König wrote: >>>> On 29.09.2015 13:40, Daniel Vetter wrote: >>>> >On Thu, Jul 09, 2015 at 12:21:06PM -0400, Alex Deucher wrote: >>>> >>From: Chunming Zhou <david1.zhou@amd.com> >>>> >> >>>> >>This implements the cgs interface for allocating >>>> >>GPU memory. >>>> >> >>>> >>Reviewed-by: Jammy Zhou <Jammy.Zhou@amd.com> >>>> >I don't see that review anywhere on a m-l ... where is it? >>>> >>>> Jammy reviewed the stuff internally before we made it public, that's why you >>>> can't find it. > > Can we get these reviews done publically? it's kinda hard to know how > well someone > reviewed things if we have no external copy. Like did Jammy a) read > the patch, and > slap Reviewed-by on it, or did he b) comment on some whitespace issues > and slap R-b > on it, or c) did he suggest a bunch of changes and those changes were > made and a new > version was produced and he r-b'ed that etc. > We are pushing for that and making steady progress, but things are slow to move internally. >>> The other stuff seems a lot more benign. For the irq abstraction >>> specifically it might be worth looking at the irq_chip stuff linux core >>> has, which is what's used to virtualize/abstract irq routing and handling. >>> But for that stuff it's a balance thing really how much you reinvent >>> wheels internally in the driver (e.g. i915 has it's own power_well stuff >>> which is pretty much just powerdomains reinvented, with less features). >>> >> >> I think that's one of the hardest things in the kernel: finding out if >> a solution already exists or not. We implemented our own version of >> mfd for our ACP audio block driver. Upon upsteaming we were alerted >> to mfd's existence and we converted the driver to use mfd. At the end >> of the day it was a lot of work for minimal gain, at least from a >> functionality perspective. I wish we had known about it sooner. I'll >> take a look at the irq_chip stuff. Thanks for the heads up! > > You say for minimal gain, but this is pretty much going to keep happening > to you with the development model you have chosen, get used to rewriting > things you consider finished and reviewed. I've said it before so I'll use this > to reiterate, your patches are only starting the process when you post them, > all the internal stuff you do is nice and all but it could all be done > externally > if you guys weren't so stuck on internal IP review. Otherwise you > should be taking > into account that this overhead will continue to exist in all your development, > and adjust schedules to suit. We do take that into account as evidenced by the multiple revisions of the ACP patch set for example. We know there may be a delta between short term deliverables and what eventually goes upstream and we take that into account. That doesn't change the overall amount of work involved. The fact is we didn't know about mfd so we didn't use it. I don't see how we could have avoided rewriting it if we didn't know about it in the first place. When we sent the patches out, we found out about it and made the appropriate changes. My point was just that we aren't the only ones this happens to. Alex
On Wed, Sep 30, 2015 at 06:10:52AM +1000, Dave Airlie wrote: > On 30 September 2015 at 01:41, Alex Deucher <alexdeucher@gmail.com> wrote: > > On Tue, Sep 29, 2015 at 11:20 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > >> On Tue, Sep 29, 2015 at 02:39:49PM +0200, Christian König wrote: > >>> On 29.09.2015 13:40, Daniel Vetter wrote: > >>> >On Thu, Jul 09, 2015 at 12:21:06PM -0400, Alex Deucher wrote: > >>> >>From: Chunming Zhou <david1.zhou@amd.com> > >>> >> > >>> >>This implements the cgs interface for allocating > >>> >>GPU memory. > >>> >> > >>> >>Reviewed-by: Jammy Zhou <Jammy.Zhou@amd.com> > >>> >I don't see that review anywhere on a m-l ... where is it? > >>> > >>> Jammy reviewed the stuff internally before we made it public, that's why you > >>> can't find it. > > Can we get these reviews done publically? it's kinda hard to know how > well someone > reviewed things if we have no external copy. Like did Jammy a) read > the patch, and > slap Reviewed-by on it, or did he b) comment on some whitespace issues > and slap R-b > on it, or c) did he suggest a bunch of changes and those changes were > made and a new > version was produced and he r-b'ed that etc. Yeah review in public is great. We can do that for almost everythingn since nowadays we have blanket approval for everything, except a short list of marketing relevant new features on new platforms. Occasionally the public review with mails don't work out though and then we have a big meeting, but even for those we try to summarize what we discussed and decided on the m-l. But without blanket approval this won't work I think. -Daniel
On Tue, Sep 29, 2015 at 04:28:13PM -0400, Alex Deucher wrote: > On Tue, Sep 29, 2015 at 4:10 PM, Dave Airlie <airlied@gmail.com> wrote: > > On 30 September 2015 at 01:41, Alex Deucher <alexdeucher@gmail.com> wrote: > >> On Tue, Sep 29, 2015 at 11:20 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > >>> On Tue, Sep 29, 2015 at 02:39:49PM +0200, Christian König wrote: > >>>> On 29.09.2015 13:40, Daniel Vetter wrote: > >>>> >On Thu, Jul 09, 2015 at 12:21:06PM -0400, Alex Deucher wrote: > >>>> >>From: Chunming Zhou <david1.zhou@amd.com> > >>>> >> > >>>> >>This implements the cgs interface for allocating > >>>> >>GPU memory. > >>>> >> > >>>> >>Reviewed-by: Jammy Zhou <Jammy.Zhou@amd.com> > >>>> >I don't see that review anywhere on a m-l ... where is it? > >>>> > >>>> Jammy reviewed the stuff internally before we made it public, that's why you > >>>> can't find it. > > > > Can we get these reviews done publically? it's kinda hard to know how > > well someone > > reviewed things if we have no external copy. Like did Jammy a) read > > the patch, and > > slap Reviewed-by on it, or did he b) comment on some whitespace issues > > and slap R-b > > on it, or c) did he suggest a bunch of changes and those changes were > > made and a new > > version was produced and he r-b'ed that etc. > > > > We are pushing for that and making steady progress, but things are > slow to move internally. > > >>> The other stuff seems a lot more benign. For the irq abstraction > >>> specifically it might be worth looking at the irq_chip stuff linux core > >>> has, which is what's used to virtualize/abstract irq routing and handling. > >>> But for that stuff it's a balance thing really how much you reinvent > >>> wheels internally in the driver (e.g. i915 has it's own power_well stuff > >>> which is pretty much just powerdomains reinvented, with less features). > >>> > >> > >> I think that's one of the hardest things in the kernel: finding out if > >> a solution already exists or not. We implemented our own version of > >> mfd for our ACP audio block driver. Upon upsteaming we were alerted > >> to mfd's existence and we converted the driver to use mfd. At the end > >> of the day it was a lot of work for minimal gain, at least from a > >> functionality perspective. I wish we had known about it sooner. I'll > >> take a look at the irq_chip stuff. Thanks for the heads up! > > > > You say for minimal gain, but this is pretty much going to keep happening > > to you with the development model you have chosen, get used to rewriting > > things you consider finished and reviewed. I've said it before so I'll use this > > to reiterate, your patches are only starting the process when you post them, > > all the internal stuff you do is nice and all but it could all be done > > externally > > if you guys weren't so stuck on internal IP review. Otherwise you > > should be taking > > into account that this overhead will continue to exist in all your development, > > and adjust schedules to suit. > > We do take that into account as evidenced by the multiple revisions of > the ACP patch set for example. We know there may be a delta between > short term deliverables and what eventually goes upstream and we take > that into account. That doesn't change the overall amount of work > involved. The fact is we didn't know about mfd so we didn't use it. > I don't see how we could have avoided rewriting it if we didn't know > about it in the first place. When we sent the patches out, we found > out about it and made the appropriate changes. My point was just that > we aren't the only ones this happens to. Discussing early designs on irc helps a lot with that. But ime irc is one step further away from just dragging engineers onto the mailing list, and discussing new stuff on irc before patches get written instead of just review is one step more. That still leaves you with the problem with knowing whom to talk to, but for modularization we have at least Thierry Reding and Laurent Pinchart with a lot of soc experience outside of drm, they tend to know what's out there. -Daniel
On Tue, Sep 29, 2015 at 11:41:56AM -0400, Alex Deucher wrote: > On Tue, Sep 29, 2015 at 11:20 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Tue, Sep 29, 2015 at 02:39:49PM +0200, Christian König wrote: > >> On 29.09.2015 13:40, Daniel Vetter wrote: > >> >On Thu, Jul 09, 2015 at 12:21:06PM -0400, Alex Deucher wrote: > >> >>From: Chunming Zhou <david1.zhou@amd.com> > >> >> > >> >>This implements the cgs interface for allocating > >> >>GPU memory. > >> >> > >> >>Reviewed-by: Jammy Zhou <Jammy.Zhou@amd.com> > >> >I don't see that review anywhere on a m-l ... where is it? > >> > >> Jammy reviewed the stuff internally before we made it public, that's why you > >> can't find it. > >> > >> > > >> >I stumbled over this patch because it adds a new dev->struct_mutex user > >> >(that should be a big warning sign) and then I somehow unrolled some giant > >> >chain of wtfs. Either I'm completely missing what this is used for or it > >> >probably should get reworked asap. > >> > >> The CGS functions aren't used at the moment because none of the components > >> depending on them made it into the kernel, but we wanted to keep it so that > >> we can get reviews on the general idea and if necessary rework it. > >> > >> In general it's an abstraction layer of device driver dependent functions > >> which enables us to share more code internally. > >> > >> We worked quite hard on trying to avoid the OS abstraction trap with this, > >> but if you think this still won't work feel free to object. > > > > The bit that made me look really is the import_gpu_mem thing, which seems > > to partially reinvent drm_prime.c. Given how tricky importing and > > import-caching is I'd really like to see that gone (and Alex said on irc > > he'd be ok with that). > > > > See attached patch. It was really only added for completeness. We > don't have any users of it at the moment. If we end up needing the > functionality in the future we can revisit it. > > > The other stuff seems a lot more benign. For the irq abstraction > > specifically it might be worth looking at the irq_chip stuff linux core > > has, which is what's used to virtualize/abstract irq routing and handling. > > But for that stuff it's a balance thing really how much you reinvent > > wheels internally in the driver (e.g. i915 has it's own power_well stuff > > which is pretty much just powerdomains reinvented, with less features). > > > > I think that's one of the hardest things in the kernel: finding out if > a solution already exists or not. We implemented our own version of > mfd for our ACP audio block driver. Upon upsteaming we were alerted > to mfd's existence and we converted the driver to use mfd. At the end > of the day it was a lot of work for minimal gain, at least from a > functionality perspective. I wish we had known about it sooner. I'll > take a look at the irq_chip stuff. Thanks for the heads up! > > Alex > > > But really I can't tell without the users whether I'd expect this to be > > hurt longterm or not for you ;-) But the import stuff is hurt for me since > > you noodle around in drm internals. And specifically I'd like to make > > modern drivers completely struct_mutex free with the goal to untangle the > > last hold-outs of that lock in the drm core. > > -Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > From 013170490a934731bb5fbc4cb8ee46421d2f240e Mon Sep 17 00:00:00 2001 > From: Alex Deucher <alexander.deucher@amd.com> > Date: Tue, 29 Sep 2015 10:35:45 -0400 > Subject: [PATCH] drm/amdgpu/cgs: remove import_gpu_mem > > It was added for completeness, but we don't have any users > for it yet. Daniel Vetter noted that it may be racy. Remove > it. > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 39 --------------------------------- > drivers/gpu/drm/amd/include/cgs_linux.h | 17 -------------- > 2 files changed, 56 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c > index 25be402..7949927 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c > @@ -208,44 +208,6 @@ static int amdgpu_cgs_alloc_gpu_mem(void *cgs_device, > return ret; > } > > -static int amdgpu_cgs_import_gpu_mem(void *cgs_device, int dmabuf_fd, > - cgs_handle_t *handle) > -{ > - CGS_FUNC_ADEV; > - int r; > - uint32_t dma_handle; > - struct drm_gem_object *obj; > - struct amdgpu_bo *bo; > - struct drm_device *dev = adev->ddev; > - struct drm_file *file_priv = NULL, *priv; > - > - mutex_lock(&dev->struct_mutex); > - list_for_each_entry(priv, &dev->filelist, lhead) { > - rcu_read_lock(); > - if (priv->pid == get_pid(task_pid(current))) > - file_priv = priv; > - rcu_read_unlock(); > - if (file_priv) > - break; > - } > - mutex_unlock(&dev->struct_mutex); > - r = dev->driver->prime_fd_to_handle(dev, > - file_priv, dmabuf_fd, > - &dma_handle); > - spin_lock(&file_priv->table_lock); > - > - /* Check if we currently have a reference on the object */ > - obj = idr_find(&file_priv->object_idr, dma_handle); > - if (obj == NULL) { > - spin_unlock(&file_priv->table_lock); > - return -EINVAL; > - } > - spin_unlock(&file_priv->table_lock); > - bo = gem_to_amdgpu_bo(obj); > - *handle = (cgs_handle_t)bo; > - return 0; > -} > - > static int amdgpu_cgs_free_gpu_mem(void *cgs_device, cgs_handle_t handle) > { > struct amdgpu_bo *obj = (struct amdgpu_bo *)handle; > @@ -846,7 +808,6 @@ static const struct cgs_ops amdgpu_cgs_ops = { > }; > > static const struct cgs_os_ops amdgpu_cgs_os_ops = { > - amdgpu_cgs_import_gpu_mem, > amdgpu_cgs_add_irq_source, > amdgpu_cgs_irq_get, > amdgpu_cgs_irq_put > diff --git a/drivers/gpu/drm/amd/include/cgs_linux.h b/drivers/gpu/drm/amd/include/cgs_linux.h > index 488642f..3b47ae3 100644 > --- a/drivers/gpu/drm/amd/include/cgs_linux.h > +++ b/drivers/gpu/drm/amd/include/cgs_linux.h > @@ -27,19 +27,6 @@ > #include "cgs_common.h" > > /** > - * cgs_import_gpu_mem() - Import dmabuf handle > - * @cgs_device: opaque device handle > - * @dmabuf_fd: DMABuf file descriptor > - * @handle: memory handle (output) > - * > - * Must be called in the process context that dmabuf_fd belongs to. > - * > - * Return: 0 on success, -errno otherwise > - */ > -typedef int (*cgs_import_gpu_mem_t)(void *cgs_device, int dmabuf_fd, > - cgs_handle_t *handle); > - > -/** > * cgs_irq_source_set_func() - Callback for enabling/disabling interrupt sources > * @private_data: private data provided to cgs_add_irq_source > * @src_id: interrupt source ID > @@ -114,16 +101,12 @@ typedef int (*cgs_irq_get_t)(void *cgs_device, unsigned src_id, unsigned type); > typedef int (*cgs_irq_put_t)(void *cgs_device, unsigned src_id, unsigned type); > > struct cgs_os_ops { > - cgs_import_gpu_mem_t import_gpu_mem; > - > /* IRQ handling */ > cgs_add_irq_source_t add_irq_source; > cgs_irq_get_t irq_get; > cgs_irq_put_t irq_put; > }; > > -#define cgs_import_gpu_mem(dev,dmabuf_fd,handle) \ > - CGS_OS_CALL(import_gpu_mem,dev,dmabuf_fd,handle) > #define cgs_add_irq_source(dev,src_id,num_types,set,handler,private_data) \ > CGS_OS_CALL(add_irq_source,dev,src_id,num_types,set,handler, \ > private_data) > -- > 1.8.3.1 >
On 30.09.2015 08:54, Daniel Vetter wrote: > On Tue, Sep 29, 2015 at 04:28:13PM -0400, Alex Deucher wrote: >> On Tue, Sep 29, 2015 at 4:10 PM, Dave Airlie <airlied@gmail.com> wrote: >>> On 30 September 2015 at 01:41, Alex Deucher <alexdeucher@gmail.com> wrote: >>>> On Tue, Sep 29, 2015 at 11:20 AM, Daniel Vetter <daniel@ffwll.ch> wrote: >>>>> On Tue, Sep 29, 2015 at 02:39:49PM +0200, Christian König wrote: >>>>>> On 29.09.2015 13:40, Daniel Vetter wrote: >>>>>>> On Thu, Jul 09, 2015 at 12:21:06PM -0400, Alex Deucher wrote: >>>>>>>> From: Chunming Zhou <david1.zhou@amd.com> >>>>>>>> >>>>>>>> This implements the cgs interface for allocating >>>>>>>> GPU memory. >>>>>>>> >>>>>>>> Reviewed-by: Jammy Zhou <Jammy.Zhou@amd.com> >>>>>>> I don't see that review anywhere on a m-l ... where is it? >>>>>> Jammy reviewed the stuff internally before we made it public, that's why you >>>>>> can't find it. >>> Can we get these reviews done publically? it's kinda hard to know how >>> well someone >>> reviewed things if we have no external copy. Like did Jammy a) read >>> the patch, and >>> slap Reviewed-by on it, or did he b) comment on some whitespace issues >>> and slap R-b >>> on it, or c) did he suggest a bunch of changes and those changes were >>> made and a new >>> version was produced and he r-b'ed that etc. >>> >> We are pushing for that and making steady progress, but things are >> slow to move internally. >> >>>>> The other stuff seems a lot more benign. For the irq abstraction >>>>> specifically it might be worth looking at the irq_chip stuff linux core >>>>> has, which is what's used to virtualize/abstract irq routing and handling. >>>>> But for that stuff it's a balance thing really how much you reinvent >>>>> wheels internally in the driver (e.g. i915 has it's own power_well stuff >>>>> which is pretty much just powerdomains reinvented, with less features). >>>>> >>>> I think that's one of the hardest things in the kernel: finding out if >>>> a solution already exists or not. We implemented our own version of >>>> mfd for our ACP audio block driver. Upon upsteaming we were alerted >>>> to mfd's existence and we converted the driver to use mfd. At the end >>>> of the day it was a lot of work for minimal gain, at least from a >>>> functionality perspective. I wish we had known about it sooner. I'll >>>> take a look at the irq_chip stuff. Thanks for the heads up! >>> You say for minimal gain, but this is pretty much going to keep happening >>> to you with the development model you have chosen, get used to rewriting >>> things you consider finished and reviewed. I've said it before so I'll use this >>> to reiterate, your patches are only starting the process when you post them, >>> all the internal stuff you do is nice and all but it could all be done >>> externally >>> if you guys weren't so stuck on internal IP review. Otherwise you >>> should be taking >>> into account that this overhead will continue to exist in all your development, >>> and adjust schedules to suit. >> We do take that into account as evidenced by the multiple revisions of >> the ACP patch set for example. We know there may be a delta between >> short term deliverables and what eventually goes upstream and we take >> that into account. That doesn't change the overall amount of work >> involved. The fact is we didn't know about mfd so we didn't use it. >> I don't see how we could have avoided rewriting it if we didn't know >> about it in the first place. When we sent the patches out, we found >> out about it and made the appropriate changes. My point was just that >> we aren't the only ones this happens to. > Discussing early designs on irc helps a lot with that. But ime irc is one > step further away from just dragging engineers onto the mailing list, and > discussing new stuff on irc before patches get written instead of just > review is one step more. That still leaves you with the problem with > knowing whom to talk to, but for modularization we have at least Thierry > Reding and Laurent Pinchart with a lot of soc experience outside of drm, > they tend to know what's out there. Yeah, completely agree. Alex and I have spend a lot of time and effort with developers previously working on closed source code to enable them to contribute to the different open source projects. Anybody who did something like that before knows that it certainly needs time for certain concepts to sink in. Especially thinks like stable interface and certain design criteria seem to be hard to get accepted. So guys feel to criticize the code whenever you think it make sense and please do so as early as possible, e.g. on the first round of patches not after ten revisions. We in turn try to get the code out as soon as possible, without keeping it stuck internally for too long. Christian. > -Daniel
From 013170490a934731bb5fbc4cb8ee46421d2f240e Mon Sep 17 00:00:00 2001 From: Alex Deucher <alexander.deucher@amd.com> Date: Tue, 29 Sep 2015 10:35:45 -0400 Subject: [PATCH] drm/amdgpu/cgs: remove import_gpu_mem It was added for completeness, but we don't have any users for it yet. Daniel Vetter noted that it may be racy. Remove it. Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 39 --------------------------------- drivers/gpu/drm/amd/include/cgs_linux.h | 17 -------------- 2 files changed, 56 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c index 25be402..7949927 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c @@ -208,44 +208,6 @@ static int amdgpu_cgs_alloc_gpu_mem(void *cgs_device, return ret; } -static int amdgpu_cgs_import_gpu_mem(void *cgs_device, int dmabuf_fd, - cgs_handle_t *handle) -{ - CGS_FUNC_ADEV; - int r; - uint32_t dma_handle; - struct drm_gem_object *obj; - struct amdgpu_bo *bo; - struct drm_device *dev = adev->ddev; - struct drm_file *file_priv = NULL, *priv; - - mutex_lock(&dev->struct_mutex); - list_for_each_entry(priv, &dev->filelist, lhead) { - rcu_read_lock(); - if (priv->pid == get_pid(task_pid(current))) - file_priv = priv; - rcu_read_unlock(); - if (file_priv) - break; - } - mutex_unlock(&dev->struct_mutex); - r = dev->driver->prime_fd_to_handle(dev, - file_priv, dmabuf_fd, - &dma_handle); - spin_lock(&file_priv->table_lock); - - /* Check if we currently have a reference on the object */ - obj = idr_find(&file_priv->object_idr, dma_handle); - if (obj == NULL) { - spin_unlock(&file_priv->table_lock); - return -EINVAL; - } - spin_unlock(&file_priv->table_lock); - bo = gem_to_amdgpu_bo(obj); - *handle = (cgs_handle_t)bo; - return 0; -} - static int amdgpu_cgs_free_gpu_mem(void *cgs_device, cgs_handle_t handle) { struct amdgpu_bo *obj = (struct amdgpu_bo *)handle; @@ -846,7 +808,6 @@ static const struct cgs_ops amdgpu_cgs_ops = { }; static const struct cgs_os_ops amdgpu_cgs_os_ops = { - amdgpu_cgs_import_gpu_mem, amdgpu_cgs_add_irq_source, amdgpu_cgs_irq_get, amdgpu_cgs_irq_put diff --git a/drivers/gpu/drm/amd/include/cgs_linux.h b/drivers/gpu/drm/amd/include/cgs_linux.h index 488642f..3b47ae3 100644 --- a/drivers/gpu/drm/amd/include/cgs_linux.h +++ b/drivers/gpu/drm/amd/include/cgs_linux.h @@ -27,19 +27,6 @@ #include "cgs_common.h" /** - * cgs_import_gpu_mem() - Import dmabuf handle - * @cgs_device: opaque device handle - * @dmabuf_fd: DMABuf file descriptor - * @handle: memory handle (output) - * - * Must be called in the process context that dmabuf_fd belongs to. - * - * Return: 0 on success, -errno otherwise - */ -typedef int (*cgs_import_gpu_mem_t)(void *cgs_device, int dmabuf_fd, - cgs_handle_t *handle); - -/** * cgs_irq_source_set_func() - Callback for enabling/disabling interrupt sources * @private_data: private data provided to cgs_add_irq_source * @src_id: interrupt source ID @@ -114,16 +101,12 @@ typedef int (*cgs_irq_get_t)(void *cgs_device, unsigned src_id, unsigned type); typedef int (*cgs_irq_put_t)(void *cgs_device, unsigned src_id, unsigned type); struct cgs_os_ops { - cgs_import_gpu_mem_t import_gpu_mem; - /* IRQ handling */ cgs_add_irq_source_t add_irq_source; cgs_irq_get_t irq_get; cgs_irq_put_t irq_put; }; -#define cgs_import_gpu_mem(dev,dmabuf_fd,handle) \ - CGS_OS_CALL(import_gpu_mem,dev,dmabuf_fd,handle) #define cgs_add_irq_source(dev,src_id,num_types,set,handler,private_data) \ CGS_OS_CALL(add_irq_source,dev,src_id,num_types,set,handler, \ private_data) -- 1.8.3.1