| Message ID | 1405029027-6085-7-git-send-email-oded.gabbay@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jul 11, 2014 at 12:50:08AM +0300, Oded Gabbay wrote: > The KFD driver should be loaded when the radeon driver is loaded and > should be finalized when the radeon driver is removed. > > This patch adds a function call to initialize kfd from radeon_init > and a function call to finalize kfd from radeon_exit. > > If the KFD driver is not present in the system, the initialize call > fails and the radeon driver continues normally. > > This patch also adds calls to probe, initialize and finalize a kfd device > per radeon device using the kgd-->kfd interface. > > Signed-off-by: Oded Gabbay <oded.gabbay@amd.com> It might be nice to allow to build radeon without HSA so i think an CONFIG_HSA should be added and have other thing depends on it. Otherwise this one is. Reviewed-by: Jérôme Glisse <jglisse@redhat.com> > --- > drivers/gpu/drm/radeon/radeon_drv.c | 6 ++++++ > drivers/gpu/drm/radeon/radeon_kms.c | 9 +++++++++ > 2 files changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c > index cb14213..88a45a0 100644 > --- a/drivers/gpu/drm/radeon/radeon_drv.c > +++ b/drivers/gpu/drm/radeon/radeon_drv.c > @@ -151,6 +151,9 @@ static inline void radeon_register_atpx_handler(void) {} > static inline void radeon_unregister_atpx_handler(void) {} > #endif > > +extern bool radeon_kfd_init(void); > +extern void radeon_kfd_fini(void); > + > int radeon_no_wb; > int radeon_modeset = -1; > int radeon_dynclks = -1; > @@ -630,12 +633,15 @@ static int __init radeon_init(void) > #endif > } > > + radeon_kfd_init(); > + > /* let modprobe override vga console setting */ > return drm_pci_init(driver, pdriver); > } > > static void __exit radeon_exit(void) > { > + radeon_kfd_fini(); > drm_pci_exit(driver, pdriver); > radeon_unregister_atpx_handler(); > } > diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c > index 35d9318..0748284 100644 > --- a/drivers/gpu/drm/radeon/radeon_kms.c > +++ b/drivers/gpu/drm/radeon/radeon_kms.c > @@ -34,6 +34,10 @@ > #include <linux/slab.h> > #include <linux/pm_runtime.h> > > +extern void radeon_kfd_device_probe(struct radeon_device *rdev); > +extern void radeon_kfd_device_init(struct radeon_device *rdev); > +extern void radeon_kfd_device_fini(struct radeon_device *rdev); > + > #if defined(CONFIG_VGA_SWITCHEROO) > bool radeon_has_atpx(void); > #else > @@ -63,6 +67,8 @@ int radeon_driver_unload_kms(struct drm_device *dev) > > pm_runtime_get_sync(dev->dev); > > + radeon_kfd_device_fini(rdev); > + > radeon_acpi_fini(rdev); > > radeon_modeset_fini(rdev); > @@ -142,6 +148,9 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags) > "Error during ACPI methods call\n"); > } > > + radeon_kfd_device_probe(rdev); > + radeon_kfd_device_init(rdev); > + > if (radeon_is_px(dev)) { > pm_runtime_use_autosuspend(dev->dev); > pm_runtime_set_autosuspend_delay(dev->dev, 5000); > -- > 1.9.1 >
On 11/07/14 19:36, Jerome Glisse wrote: > On Fri, Jul 11, 2014 at 12:50:08AM +0300, Oded Gabbay wrote: >> The KFD driver should be loaded when the radeon driver is loaded and >> should be finalized when the radeon driver is removed. >> >> This patch adds a function call to initialize kfd from radeon_init >> and a function call to finalize kfd from radeon_exit. >> >> If the KFD driver is not present in the system, the initialize call >> fails and the radeon driver continues normally. >> >> This patch also adds calls to probe, initialize and finalize a kfd device >> per radeon device using the kgd-->kfd interface. >> >> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com> > > It might be nice to allow to build radeon without HSA so i think an > CONFIG_HSA should be added and have other thing depends on it. > Otherwise this one is. > > Reviewed-by: Jérôme Glisse <jglisse@redhat.com> > We do allow it :) There is no problem building radeon without the kfd. In that case, when radeon finds out that kfd is not available, it simply moves on with its initialization procedure. Oded > >> --- >> drivers/gpu/drm/radeon/radeon_drv.c | 6 ++++++ >> drivers/gpu/drm/radeon/radeon_kms.c | 9 +++++++++ >> 2 files changed, 15 insertions(+) >> >> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c >> index cb14213..88a45a0 100644 >> --- a/drivers/gpu/drm/radeon/radeon_drv.c >> +++ b/drivers/gpu/drm/radeon/radeon_drv.c >> @@ -151,6 +151,9 @@ static inline void radeon_register_atpx_handler(void) {} >> static inline void radeon_unregister_atpx_handler(void) {} >> #endif >> >> +extern bool radeon_kfd_init(void); >> +extern void radeon_kfd_fini(void); >> + >> int radeon_no_wb; >> int radeon_modeset = -1; >> int radeon_dynclks = -1; >> @@ -630,12 +633,15 @@ static int __init radeon_init(void) >> #endif >> } >> >> + radeon_kfd_init(); >> + >> /* let modprobe override vga console setting */ >> return drm_pci_init(driver, pdriver); >> } >> >> static void __exit radeon_exit(void) >> { >> + radeon_kfd_fini(); >> drm_pci_exit(driver, pdriver); >> radeon_unregister_atpx_handler(); >> } >> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c >> index 35d9318..0748284 100644 >> --- a/drivers/gpu/drm/radeon/radeon_kms.c >> +++ b/drivers/gpu/drm/radeon/radeon_kms.c >> @@ -34,6 +34,10 @@ >> #include <linux/slab.h> >> #include <linux/pm_runtime.h> >> >> +extern void radeon_kfd_device_probe(struct radeon_device *rdev); >> +extern void radeon_kfd_device_init(struct radeon_device *rdev); >> +extern void radeon_kfd_device_fini(struct radeon_device *rdev); >> + >> #if defined(CONFIG_VGA_SWITCHEROO) >> bool radeon_has_atpx(void); >> #else >> @@ -63,6 +67,8 @@ int radeon_driver_unload_kms(struct drm_device *dev) >> >> pm_runtime_get_sync(dev->dev); >> >> + radeon_kfd_device_fini(rdev); >> + >> radeon_acpi_fini(rdev); >> >> radeon_modeset_fini(rdev); >> @@ -142,6 +148,9 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags) >> "Error during ACPI methods call\n"); >> } >> >> + radeon_kfd_device_probe(rdev); >> + radeon_kfd_device_init(rdev); >> + >> if (radeon_is_px(dev)) { >> pm_runtime_use_autosuspend(dev->dev); >> pm_runtime_set_autosuspend_delay(dev->dev, 5000); >> -- >> 1.9.1 >>
Am 17.07.2014 13:57, schrieb Oded Gabbay: > On 11/07/14 19:36, Jerome Glisse wrote: >> On Fri, Jul 11, 2014 at 12:50:08AM +0300, Oded Gabbay wrote: >>> The KFD driver should be loaded when the radeon driver is loaded and >>> should be finalized when the radeon driver is removed. >>> >>> This patch adds a function call to initialize kfd from radeon_init >>> and a function call to finalize kfd from radeon_exit. >>> >>> If the KFD driver is not present in the system, the initialize call >>> fails and the radeon driver continues normally. >>> >>> This patch also adds calls to probe, initialize and finalize a kfd >>> device >>> per radeon device using the kgd-->kfd interface. >>> >>> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com> >> >> It might be nice to allow to build radeon without HSA so i think an >> CONFIG_HSA should be added and have other thing depends on it. >> Otherwise this one is. >> >> Reviewed-by: Jérôme Glisse <jglisse@redhat.com> >> > We do allow it :) > There is no problem building radeon without the kfd. In that case, > when radeon finds out that kfd is not available, it simply moves on > with its initialization procedure. At least off hand I don't see how this should work. Radeon directly calls radeon_kfd_(probe|init|fini) and so has a direct dependency on it. Christian. > > Oded >> >>> --- >>> drivers/gpu/drm/radeon/radeon_drv.c | 6 ++++++ >>> drivers/gpu/drm/radeon/radeon_kms.c | 9 +++++++++ >>> 2 files changed, 15 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c >>> b/drivers/gpu/drm/radeon/radeon_drv.c >>> index cb14213..88a45a0 100644 >>> --- a/drivers/gpu/drm/radeon/radeon_drv.c >>> +++ b/drivers/gpu/drm/radeon/radeon_drv.c >>> @@ -151,6 +151,9 @@ static inline void >>> radeon_register_atpx_handler(void) {} >>> static inline void radeon_unregister_atpx_handler(void) {} >>> #endif >>> >>> +extern bool radeon_kfd_init(void); >>> +extern void radeon_kfd_fini(void); >>> + >>> int radeon_no_wb; >>> int radeon_modeset = -1; >>> int radeon_dynclks = -1; >>> @@ -630,12 +633,15 @@ static int __init radeon_init(void) >>> #endif >>> } >>> >>> + radeon_kfd_init(); >>> + >>> /* let modprobe override vga console setting */ >>> return drm_pci_init(driver, pdriver); >>> } >>> >>> static void __exit radeon_exit(void) >>> { >>> + radeon_kfd_fini(); >>> drm_pci_exit(driver, pdriver); >>> radeon_unregister_atpx_handler(); >>> } >>> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c >>> b/drivers/gpu/drm/radeon/radeon_kms.c >>> index 35d9318..0748284 100644 >>> --- a/drivers/gpu/drm/radeon/radeon_kms.c >>> +++ b/drivers/gpu/drm/radeon/radeon_kms.c >>> @@ -34,6 +34,10 @@ >>> #include <linux/slab.h> >>> #include <linux/pm_runtime.h> >>> >>> +extern void radeon_kfd_device_probe(struct radeon_device *rdev); >>> +extern void radeon_kfd_device_init(struct radeon_device *rdev); >>> +extern void radeon_kfd_device_fini(struct radeon_device *rdev); >>> + >>> #if defined(CONFIG_VGA_SWITCHEROO) >>> bool radeon_has_atpx(void); >>> #else >>> @@ -63,6 +67,8 @@ int radeon_driver_unload_kms(struct drm_device *dev) >>> >>> pm_runtime_get_sync(dev->dev); >>> >>> + radeon_kfd_device_fini(rdev); >>> + >>> radeon_acpi_fini(rdev); >>> >>> radeon_modeset_fini(rdev); >>> @@ -142,6 +148,9 @@ int radeon_driver_load_kms(struct drm_device >>> *dev, unsigned long flags) >>> "Error during ACPI methods call\n"); >>> } >>> >>> + radeon_kfd_device_probe(rdev); >>> + radeon_kfd_device_init(rdev); >>> + >>> if (radeon_is_px(dev)) { >>> pm_runtime_use_autosuspend(dev->dev); >>> pm_runtime_set_autosuspend_delay(dev->dev, 5000); >>> -- >>> 1.9.1 >>> >
On 17/07/14 15:29, Christian König wrote: > Am 17.07.2014 13:57, schrieb Oded Gabbay: >> On 11/07/14 19:36, Jerome Glisse wrote: >>> On Fri, Jul 11, 2014 at 12:50:08AM +0300, Oded Gabbay wrote: >>>> The KFD driver should be loaded when the radeon driver is loaded and >>>> should be finalized when the radeon driver is removed. >>>> >>>> This patch adds a function call to initialize kfd from radeon_init >>>> and a function call to finalize kfd from radeon_exit. >>>> >>>> If the KFD driver is not present in the system, the initialize call >>>> fails and the radeon driver continues normally. >>>> >>>> This patch also adds calls to probe, initialize and finalize a kfd device >>>> per radeon device using the kgd-->kfd interface. >>>> >>>> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com> >>> >>> It might be nice to allow to build radeon without HSA so i think an >>> CONFIG_HSA should be added and have other thing depends on it. >>> Otherwise this one is. >>> >>> Reviewed-by: Jérôme Glisse <jglisse@redhat.com> >>> >> We do allow it :) >> There is no problem building radeon without the kfd. In that case, when radeon >> finds out that kfd is not available, it simply moves on with its >> initialization procedure. > > At least off hand I don't see how this should work. Radeon directly calls > radeon_kfd_(probe|init|fini) and so has a direct dependency on it. > > Christian. But radeon_kfd.c is now a permanent part of the radeon driver. I talked with Alex about it and we both agreed on that. So radeon_kfd_* functions are *always* there when you build radeon. Oded > >> >> Oded >>> >>>> --- >>>> drivers/gpu/drm/radeon/radeon_drv.c | 6 ++++++ >>>> drivers/gpu/drm/radeon/radeon_kms.c | 9 +++++++++ >>>> 2 files changed, 15 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c >>>> b/drivers/gpu/drm/radeon/radeon_drv.c >>>> index cb14213..88a45a0 100644 >>>> --- a/drivers/gpu/drm/radeon/radeon_drv.c >>>> +++ b/drivers/gpu/drm/radeon/radeon_drv.c >>>> @@ -151,6 +151,9 @@ static inline void radeon_register_atpx_handler(void) {} >>>> static inline void radeon_unregister_atpx_handler(void) {} >>>> #endif >>>> >>>> +extern bool radeon_kfd_init(void); >>>> +extern void radeon_kfd_fini(void); >>>> + >>>> int radeon_no_wb; >>>> int radeon_modeset = -1; >>>> int radeon_dynclks = -1; >>>> @@ -630,12 +633,15 @@ static int __init radeon_init(void) >>>> #endif >>>> } >>>> >>>> + radeon_kfd_init(); >>>> + >>>> /* let modprobe override vga console setting */ >>>> return drm_pci_init(driver, pdriver); >>>> } >>>> >>>> static void __exit radeon_exit(void) >>>> { >>>> + radeon_kfd_fini(); >>>> drm_pci_exit(driver, pdriver); >>>> radeon_unregister_atpx_handler(); >>>> } >>>> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c >>>> b/drivers/gpu/drm/radeon/radeon_kms.c >>>> index 35d9318..0748284 100644 >>>> --- a/drivers/gpu/drm/radeon/radeon_kms.c >>>> +++ b/drivers/gpu/drm/radeon/radeon_kms.c >>>> @@ -34,6 +34,10 @@ >>>> #include <linux/slab.h> >>>> #include <linux/pm_runtime.h> >>>> >>>> +extern void radeon_kfd_device_probe(struct radeon_device *rdev); >>>> +extern void radeon_kfd_device_init(struct radeon_device *rdev); >>>> +extern void radeon_kfd_device_fini(struct radeon_device *rdev); >>>> + >>>> #if defined(CONFIG_VGA_SWITCHEROO) >>>> bool radeon_has_atpx(void); >>>> #else >>>> @@ -63,6 +67,8 @@ int radeon_driver_unload_kms(struct drm_device *dev) >>>> >>>> pm_runtime_get_sync(dev->dev); >>>> >>>> + radeon_kfd_device_fini(rdev); >>>> + >>>> radeon_acpi_fini(rdev); >>>> >>>> radeon_modeset_fini(rdev); >>>> @@ -142,6 +148,9 @@ int radeon_driver_load_kms(struct drm_device *dev, >>>> unsigned long flags) >>>> "Error during ACPI methods call\n"); >>>> } >>>> >>>> + radeon_kfd_device_probe(rdev); >>>> + radeon_kfd_device_init(rdev); >>>> + >>>> if (radeon_is_px(dev)) { >>>> pm_runtime_use_autosuspend(dev->dev); >>>> pm_runtime_set_autosuspend_delay(dev->dev, 5000); >>>> -- >>>> 1.9.1 >>>> >> >
Am 17.07.2014 14:30, schrieb Oded Gabbay: > On 17/07/14 15:29, Christian König wrote: >> Am 17.07.2014 13:57, schrieb Oded Gabbay: >>> On 11/07/14 19:36, Jerome Glisse wrote: >>>> On Fri, Jul 11, 2014 at 12:50:08AM +0300, Oded Gabbay wrote: >>>>> The KFD driver should be loaded when the radeon driver is loaded and >>>>> should be finalized when the radeon driver is removed. >>>>> >>>>> This patch adds a function call to initialize kfd from radeon_init >>>>> and a function call to finalize kfd from radeon_exit. >>>>> >>>>> If the KFD driver is not present in the system, the initialize call >>>>> fails and the radeon driver continues normally. >>>>> >>>>> This patch also adds calls to probe, initialize and finalize a kfd >>>>> device >>>>> per radeon device using the kgd-->kfd interface. >>>>> >>>>> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com> >>>> >>>> It might be nice to allow to build radeon without HSA so i think an >>>> CONFIG_HSA should be added and have other thing depends on it. >>>> Otherwise this one is. >>>> >>>> Reviewed-by: Jérôme Glisse <jglisse@redhat.com> >>>> >>> We do allow it :) >>> There is no problem building radeon without the kfd. In that case, >>> when radeon >>> finds out that kfd is not available, it simply moves on with its >>> initialization procedure. >> >> At least off hand I don't see how this should work. Radeon directly >> calls >> radeon_kfd_(probe|init|fini) and so has a direct dependency on it. >> >> Christian. > But radeon_kfd.c is now a permanent part of the radeon driver. I > talked with Alex about it and we both agreed on that. So radeon_kfd_* > functions are *always* there when you build radeon. Ah, I see. So radeon_kfd_init then tries to load the other module through symbol_request(). Long story short that's a bad idea for a couple of reasons. First of all it only works when you build everything as module and second by doing so the radeon<->kfd interface must be handled as internal stable interface. Only a very few drivers/subsystem do use symbol_request() and to see how to use it correctly please take a look at (for example) sound/pci/hda/hda_codec.c. Essentially you need to handle all different combination of module vs. builtin like this: > 1660 <http://lxr.free-electrons.com/source/sound/pci/hda/hda_codec.c#L1660> #ifIS_MODULE <http://lxr.free-electrons.com/ident?i=IS_MODULE>(CONFIG_SND_HDA_GENERIC) > 1661 <http://lxr.free-electrons.com/source/sound/pci/hda/hda_codec.c#L1661> patch <http://lxr.free-electrons.com/ident?i=patch> =load_parser <http://lxr.free-electrons.com/ident?i=load_parser>(codec <http://lxr.free-electrons.com/ident?i=codec>,snd_hda_parse_generic_codec <http://lxr.free-electrons.com/ident?i=snd_hda_parse_generic_codec>); > 1662 <http://lxr.free-electrons.com/source/sound/pci/hda/hda_codec.c#L1662> #elifIS_BUILTIN <http://lxr.free-electrons.com/ident?i=IS_BUILTIN>(CONFIG_SND_HDA_GENERIC) > 1663 <http://lxr.free-electrons.com/source/sound/pci/hda/hda_codec.c#L1663> patch <http://lxr.free-electrons.com/ident?i=patch> =snd_hda_parse_generic_codec <http://lxr.free-electrons.com/ident?i=snd_hda_parse_generic_codec>; > 1664 <http://lxr.free-electrons.com/source/sound/pci/hda/hda_codec.c#L1664> #endif I strongly suggest to just make the radeon module depend directly on the KFD module through a CONFIG_RADEON_KFD option. Regards, Christian. > Oded >> >>> >>> Oded >>>> >>>>> --- >>>>> drivers/gpu/drm/radeon/radeon_drv.c | 6 ++++++ >>>>> drivers/gpu/drm/radeon/radeon_kms.c | 9 +++++++++ >>>>> 2 files changed, 15 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c >>>>> b/drivers/gpu/drm/radeon/radeon_drv.c >>>>> index cb14213..88a45a0 100644 >>>>> --- a/drivers/gpu/drm/radeon/radeon_drv.c >>>>> +++ b/drivers/gpu/drm/radeon/radeon_drv.c >>>>> @@ -151,6 +151,9 @@ static inline void >>>>> radeon_register_atpx_handler(void) {} >>>>> static inline void radeon_unregister_atpx_handler(void) {} >>>>> #endif >>>>> >>>>> +extern bool radeon_kfd_init(void); >>>>> +extern void radeon_kfd_fini(void); >>>>> + >>>>> int radeon_no_wb; >>>>> int radeon_modeset = -1; >>>>> int radeon_dynclks = -1; >>>>> @@ -630,12 +633,15 @@ static int __init radeon_init(void) >>>>> #endif >>>>> } >>>>> >>>>> + radeon_kfd_init(); >>>>> + >>>>> /* let modprobe override vga console setting */ >>>>> return drm_pci_init(driver, pdriver); >>>>> } >>>>> >>>>> static void __exit radeon_exit(void) >>>>> { >>>>> + radeon_kfd_fini(); >>>>> drm_pci_exit(driver, pdriver); >>>>> radeon_unregister_atpx_handler(); >>>>> } >>>>> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c >>>>> b/drivers/gpu/drm/radeon/radeon_kms.c >>>>> index 35d9318..0748284 100644 >>>>> --- a/drivers/gpu/drm/radeon/radeon_kms.c >>>>> +++ b/drivers/gpu/drm/radeon/radeon_kms.c >>>>> @@ -34,6 +34,10 @@ >>>>> #include <linux/slab.h> >>>>> #include <linux/pm_runtime.h> >>>>> >>>>> +extern void radeon_kfd_device_probe(struct radeon_device *rdev); >>>>> +extern void radeon_kfd_device_init(struct radeon_device *rdev); >>>>> +extern void radeon_kfd_device_fini(struct radeon_device *rdev); >>>>> + >>>>> #if defined(CONFIG_VGA_SWITCHEROO) >>>>> bool radeon_has_atpx(void); >>>>> #else >>>>> @@ -63,6 +67,8 @@ int radeon_driver_unload_kms(struct drm_device >>>>> *dev) >>>>> >>>>> pm_runtime_get_sync(dev->dev); >>>>> >>>>> + radeon_kfd_device_fini(rdev); >>>>> + >>>>> radeon_acpi_fini(rdev); >>>>> >>>>> radeon_modeset_fini(rdev); >>>>> @@ -142,6 +148,9 @@ int radeon_driver_load_kms(struct drm_device >>>>> *dev, >>>>> unsigned long flags) >>>>> "Error during ACPI methods call\n"); >>>>> } >>>>> >>>>> + radeon_kfd_device_probe(rdev); >>>>> + radeon_kfd_device_init(rdev); >>>>> + >>>>> if (radeon_is_px(dev)) { >>>>> pm_runtime_use_autosuspend(dev->dev); >>>>> pm_runtime_set_autosuspend_delay(dev->dev, 5000); >>>>> -- >>>>> 1.9.1 >>>>> >>> >> >
On Thu, Jul 17, 2014 at 02:45:09PM +0200, Christian König wrote: > Am 17.07.2014 14:30, schrieb Oded Gabbay: > >On 17/07/14 15:29, Christian König wrote: > >>Am 17.07.2014 13:57, schrieb Oded Gabbay: > >>>On 11/07/14 19:36, Jerome Glisse wrote: > >>>>On Fri, Jul 11, 2014 at 12:50:08AM +0300, Oded Gabbay wrote: > >>>>>The KFD driver should be loaded when the radeon driver is loaded and > >>>>>should be finalized when the radeon driver is removed. > >>>>> > >>>>>This patch adds a function call to initialize kfd from radeon_init > >>>>>and a function call to finalize kfd from radeon_exit. > >>>>> > >>>>>If the KFD driver is not present in the system, the initialize call > >>>>>fails and the radeon driver continues normally. > >>>>> > >>>>>This patch also adds calls to probe, initialize and finalize a kfd > >>>>>device > >>>>>per radeon device using the kgd-->kfd interface. > >>>>> > >>>>>Signed-off-by: Oded Gabbay <oded.gabbay@amd.com> > >>>> > >>>>It might be nice to allow to build radeon without HSA so i think an > >>>>CONFIG_HSA should be added and have other thing depends on it. > >>>>Otherwise this one is. > >>>> > >>>>Reviewed-by: Jérôme Glisse <jglisse@redhat.com> > >>>> > >>>We do allow it :) > >>>There is no problem building radeon without the kfd. In that case, > >>>when radeon > >>>finds out that kfd is not available, it simply moves on with its > >>>initialization procedure. > >> > >>At least off hand I don't see how this should work. Radeon directly > >>calls > >>radeon_kfd_(probe|init|fini) and so has a direct dependency on it. > >> > >>Christian. > >But radeon_kfd.c is now a permanent part of the radeon driver. I talked > >with Alex about it and we both agreed on that. So radeon_kfd_* functions > >are *always* there when you build radeon. > > Ah, I see. So radeon_kfd_init then tries to load the other module through > symbol_request(). Long story short that's a bad idea for a couple of > reasons. > > First of all it only works when you build everything as module and second by > doing so the radeon<->kfd interface must be handled as internal stable > interface. > > Only a very few drivers/subsystem do use symbol_request() and to see how to > use it correctly please take a look at (for example) > sound/pci/hda/hda_codec.c. We do this in i915 to coordinate a bunch of things with the snd_hda driver. And it's a major pain. Imo the proper way to do this is for one driver to expose a platform driver with a bunch of specific interfaces and for the other driver to register as a platform driver against that device. Then all the usual linux hotplug infrastructure will make sure that this all works and there's a clear runtime depency. For i915 that's what I've requested the audio guys to look into, and also what I'll require for other such sub-driver stuff (e.g. we have a non-intel video codec on vlv gfx). Well for audio it will be a bit fancier since we also want some standardized stuff to allow userspace to see the association between the gfx output and the audio side. Atm you can only guess if you have more than one screen connected. This approach gives you full flexibility and you can e.g. blacklist the subdriver for debugging, without a kernel recompile. -Daniel
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index cb14213..88a45a0 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -151,6 +151,9 @@ static inline void radeon_register_atpx_handler(void) {} static inline void radeon_unregister_atpx_handler(void) {} #endif +extern bool radeon_kfd_init(void); +extern void radeon_kfd_fini(void); + int radeon_no_wb; int radeon_modeset = -1; int radeon_dynclks = -1; @@ -630,12 +633,15 @@ static int __init radeon_init(void) #endif } + radeon_kfd_init(); + /* let modprobe override vga console setting */ return drm_pci_init(driver, pdriver); } static void __exit radeon_exit(void) { + radeon_kfd_fini(); drm_pci_exit(driver, pdriver); radeon_unregister_atpx_handler(); } diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c index 35d9318..0748284 100644 --- a/drivers/gpu/drm/radeon/radeon_kms.c +++ b/drivers/gpu/drm/radeon/radeon_kms.c @@ -34,6 +34,10 @@ #include <linux/slab.h> #include <linux/pm_runtime.h> +extern void radeon_kfd_device_probe(struct radeon_device *rdev); +extern void radeon_kfd_device_init(struct radeon_device *rdev); +extern void radeon_kfd_device_fini(struct radeon_device *rdev); + #if defined(CONFIG_VGA_SWITCHEROO) bool radeon_has_atpx(void); #else @@ -63,6 +67,8 @@ int radeon_driver_unload_kms(struct drm_device *dev) pm_runtime_get_sync(dev->dev); + radeon_kfd_device_fini(rdev); + radeon_acpi_fini(rdev); radeon_modeset_fini(rdev); @@ -142,6 +148,9 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags) "Error during ACPI methods call\n"); } + radeon_kfd_device_probe(rdev); + radeon_kfd_device_init(rdev); + if (radeon_is_px(dev)) { pm_runtime_use_autosuspend(dev->dev); pm_runtime_set_autosuspend_delay(dev->dev, 5000);
The KFD driver should be loaded when the radeon driver is loaded and should be finalized when the radeon driver is removed. This patch adds a function call to initialize kfd from radeon_init and a function call to finalize kfd from radeon_exit. If the KFD driver is not present in the system, the initialize call fails and the radeon driver continues normally. This patch also adds calls to probe, initialize and finalize a kfd device per radeon device using the kgd-->kfd interface. Signed-off-by: Oded Gabbay <oded.gabbay@amd.com> --- drivers/gpu/drm/radeon/radeon_drv.c | 6 ++++++ drivers/gpu/drm/radeon/radeon_kms.c | 9 +++++++++ 2 files changed, 15 insertions(+)