From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============3969009718562887363==" MIME-Version: 1.0 From: Christian König To: lkp@lists.01.org Subject: Re: [PATCH v2] drm/radeon: Init amdkfd only if it was compiled Date: Mon, 29 Dec 2014 13:41:48 +0100 Message-ID: <54A14C0C.8090401@vodafone.de> In-Reply-To: <1419843586-29864-1-git-send-email-oded.gabbay@amd.com> List-Id: --===============3969009718562887363== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Am 29.12.2014 um 09:59 schrieb Oded Gabbay: > This patch changes the radeon_kfd_init(), which is used to initialize the > interface between radeon and amdkfd, so the interface will be initialized= only > if amdkfd was build, either as module or inside the kernel image. > > In the modules case, the symbol_request() will be used (same as old code)= . In > the in-image compilation case, a direct call to kgd2kfd_init() will be do= ne. > For other cases, radeon_kfd_init() will just return false. > > This patch is necessary because in case of the following specific > configuration: kernel 32-bit, no modules support, random kernel base and = no > hibernation, the symbol_request() doesn't work as expected - it doesn't r= eturn > NULL if the symbol doesn't exists - which makes the kernel panic. > > Signed-off-by: Oded Gabbay Using symbol_request() in general doesn't seem to be such a good idea, = because it breaks symbol versioning and as far as I can see is only very = rarely used in drivers (AFAIK only V4L drivers). But for the moment I don't have another idea either except for always = loading the module which on most hardware is completely unnecessary. = Maybe we should consider restructuring the module load dependencies in = the next kernel release. For now the patch is Reviewed-by: Christian K=C3=B6nig > --- > drivers/gpu/drm/radeon/radeon_kfd.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/radeon/radeon_kfd.c b/drivers/gpu/drm/radeon= /radeon_kfd.c > index 242fd8b..d3e78b4 100644 > --- a/drivers/gpu/drm/radeon/radeon_kfd.c > +++ b/drivers/gpu/drm/radeon/radeon_kfd.c > @@ -101,6 +101,7 @@ static const struct kgd2kfd_calls *kgd2kfd; > = > bool radeon_kfd_init(void) > { > +#if defined(CONFIG_HSA_AMD_MODULE) > bool (*kgd2kfd_init_p)(unsigned, const struct kfd2kgd_calls*, > const struct kgd2kfd_calls**); > = > @@ -117,6 +118,17 @@ bool radeon_kfd_init(void) > } > = > return true; > +#elif defined(CONFIG_HSA_AMD) > + if (!kgd2kfd_init(KFD_INTERFACE_VERSION, &kfd2kgd, &kgd2kfd)) { > + kgd2kfd =3D NULL; > + > + return false; > + } > + > + return true; > +#else > + return false; > +#endif > } > = > void radeon_kfd_fini(void) --===============3969009718562887363==-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-15?Q?Christian_K=F6nig?= Subject: Re: [PATCH v2] drm/radeon: Init amdkfd only if it was compiled Date: Mon, 29 Dec 2014 13:41:48 +0100 Message-ID: <54A14C0C.8090401@vodafone.de> References: <1419843586-29864-1-git-send-email-oded.gabbay@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: Received: from pegasos-out.vodafone.de (pegasos-out.vodafone.de [80.84.1.38]) by gabe.freedesktop.org (Postfix) with ESMTP id 8F94189DD5 for ; Mon, 29 Dec 2014 04:42:26 -0800 (PST) In-Reply-To: <1419843586-29864-1-git-send-email-oded.gabbay@amd.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Oded Gabbay , dri-devel@lists.freedesktop.org, Alexander Deucher Cc: ak@linux.intel.com, rusty@rustcorp.com.au, linux-kernel@vger.kernel.org, Huang Ying , LKP ML List-Id: dri-devel@lists.freedesktop.org QW0gMjkuMTIuMjAxNCB1bSAwOTo1OSBzY2hyaWViIE9kZWQgR2FiYmF5Ogo+IFRoaXMgcGF0Y2gg Y2hhbmdlcyB0aGUgcmFkZW9uX2tmZF9pbml0KCksIHdoaWNoIGlzIHVzZWQgdG8gaW5pdGlhbGl6 ZSB0aGUKPiBpbnRlcmZhY2UgYmV0d2VlbiByYWRlb24gYW5kIGFtZGtmZCwgc28gdGhlIGludGVy ZmFjZSB3aWxsIGJlIGluaXRpYWxpemVkIG9ubHkKPiBpZiBhbWRrZmQgd2FzIGJ1aWxkLCBlaXRo ZXIgYXMgbW9kdWxlIG9yIGluc2lkZSB0aGUga2VybmVsIGltYWdlLgo+Cj4gSW4gdGhlIG1vZHVs ZXMgY2FzZSwgdGhlIHN5bWJvbF9yZXF1ZXN0KCkgd2lsbCBiZSB1c2VkIChzYW1lIGFzIG9sZCBj b2RlKS4gSW4KPiB0aGUgaW4taW1hZ2UgY29tcGlsYXRpb24gY2FzZSwgYSBkaXJlY3QgY2FsbCB0 byBrZ2Qya2ZkX2luaXQoKSB3aWxsIGJlIGRvbmUuCj4gRm9yIG90aGVyIGNhc2VzLCByYWRlb25f a2ZkX2luaXQoKSB3aWxsIGp1c3QgcmV0dXJuIGZhbHNlLgo+Cj4gVGhpcyBwYXRjaCBpcyBuZWNl c3NhcnkgYmVjYXVzZSBpbiBjYXNlIG9mIHRoZSBmb2xsb3dpbmcgc3BlY2lmaWMKPiBjb25maWd1 cmF0aW9uOiBrZXJuZWwgMzItYml0LCBubyBtb2R1bGVzIHN1cHBvcnQsIHJhbmRvbSBrZXJuZWwg YmFzZSBhbmQgbm8KPiBoaWJlcm5hdGlvbiwgdGhlIHN5bWJvbF9yZXF1ZXN0KCkgZG9lc24ndCB3 b3JrIGFzIGV4cGVjdGVkIC0gaXQgZG9lc24ndCByZXR1cm4KPiBOVUxMIGlmIHRoZSBzeW1ib2wg ZG9lc24ndCBleGlzdHMgLSB3aGljaCBtYWtlcyB0aGUga2VybmVsIHBhbmljLgo+Cj4gU2lnbmVk LW9mZi1ieTogT2RlZCBHYWJiYXkgPG9kZWQuZ2FiYmF5QGFtZC5jb20+CgpVc2luZyBzeW1ib2xf cmVxdWVzdCgpIGluIGdlbmVyYWwgZG9lc24ndCBzZWVtIHRvIGJlIHN1Y2ggYSBnb29kIGlkZWEs IApiZWNhdXNlIGl0IGJyZWFrcyBzeW1ib2wgdmVyc2lvbmluZyBhbmQgYXMgZmFyIGFzIEkgY2Fu IHNlZSBpcyBvbmx5IHZlcnkgCnJhcmVseSB1c2VkIGluIGRyaXZlcnMgKEFGQUlLIG9ubHkgVjRM IGRyaXZlcnMpLgoKQnV0IGZvciB0aGUgbW9tZW50IEkgZG9uJ3QgaGF2ZSBhbm90aGVyIGlkZWEg ZWl0aGVyIGV4Y2VwdCBmb3IgYWx3YXlzIApsb2FkaW5nIHRoZSBtb2R1bGUgd2hpY2ggb24gbW9z dCBoYXJkd2FyZSBpcyBjb21wbGV0ZWx5IHVubmVjZXNzYXJ5LiAKTWF5YmUgd2Ugc2hvdWxkIGNv bnNpZGVyIHJlc3RydWN0dXJpbmcgdGhlIG1vZHVsZSBsb2FkIGRlcGVuZGVuY2llcyBpbiAKdGhl IG5leHQga2VybmVsIHJlbGVhc2UuCgpGb3Igbm93IHRoZSBwYXRjaCBpcyBSZXZpZXdlZC1ieTog Q2hyaXN0aWFuIEvDtm5pZyA8Y2hyaXN0aWFuLmtvZW5pZ0BhbWQuY29tPgoKPiAtLS0KPiAgIGRy aXZlcnMvZ3B1L2RybS9yYWRlb24vcmFkZW9uX2tmZC5jIHwgMTIgKysrKysrKysrKysrCj4gICAx IGZpbGUgY2hhbmdlZCwgMTIgaW5zZXJ0aW9ucygrKQo+Cj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMv Z3B1L2RybS9yYWRlb24vcmFkZW9uX2tmZC5jIGIvZHJpdmVycy9ncHUvZHJtL3JhZGVvbi9yYWRl b25fa2ZkLmMKPiBpbmRleCAyNDJmZDhiLi5kM2U3OGI0IDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMv Z3B1L2RybS9yYWRlb24vcmFkZW9uX2tmZC5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL3JhZGVv bi9yYWRlb25fa2ZkLmMKPiBAQCAtMTAxLDYgKzEwMSw3IEBAIHN0YXRpYyBjb25zdCBzdHJ1Y3Qg a2dkMmtmZF9jYWxscyAqa2dkMmtmZDsKPiAgIAo+ICAgYm9vbCByYWRlb25fa2ZkX2luaXQodm9p ZCkKPiAgIHsKPiArI2lmIGRlZmluZWQoQ09ORklHX0hTQV9BTURfTU9EVUxFKQo+ICAgCWJvb2wg KCprZ2Qya2ZkX2luaXRfcCkodW5zaWduZWQsIGNvbnN0IHN0cnVjdCBrZmQya2dkX2NhbGxzKiwK PiAgIAkJCQljb25zdCBzdHJ1Y3Qga2dkMmtmZF9jYWxscyoqKTsKPiAgIAo+IEBAIC0xMTcsNiAr MTE4LDE3IEBAIGJvb2wgcmFkZW9uX2tmZF9pbml0KHZvaWQpCj4gICAJfQo+ICAgCj4gICAJcmV0 dXJuIHRydWU7Cj4gKyNlbGlmIGRlZmluZWQoQ09ORklHX0hTQV9BTUQpCj4gKwlpZiAoIWtnZDJr ZmRfaW5pdChLRkRfSU5URVJGQUNFX1ZFUlNJT04sICZrZmQya2dkLCAma2dkMmtmZCkpIHsKPiAr CQlrZ2Qya2ZkID0gTlVMTDsKPiArCj4gKwkJcmV0dXJuIGZhbHNlOwo+ICsJfQo+ICsKPiArCXJl dHVybiB0cnVlOwo+ICsjZWxzZQo+ICsJcmV0dXJuIGZhbHNlOwo+ICsjZW5kaWYKPiAgIH0KPiAg IAo+ICAgdm9pZCByYWRlb25fa2ZkX2Zpbmkodm9pZCkKCl9fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVs QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxt YW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752108AbaL2Mm3 (ORCPT ); Mon, 29 Dec 2014 07:42:29 -0500 Received: from pegasos-out.vodafone.de ([80.84.1.38]:52624 "EHLO pegasos-out.vodafone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751200AbaL2Mm1 (ORCPT ); Mon, 29 Dec 2014 07:42:27 -0500 X-Spam-Flag: NO X-Spam-Score: 0.157 Authentication-Results: rohrpostix2.prod.vfnet.de (amavisd-new); dkim=softfail (invalid, public key: DNS query timeout for mail._domainkey.vodafone.de) header.i=@vodafone.de X-DKIM: OpenDKIM Filter v2.6.8 pegasos-out.vodafone.de 6184F600098 Message-ID: <54A14C0C.8090401@vodafone.de> Date: Mon, 29 Dec 2014 13:41:48 +0100 From: =?ISO-8859-15?Q?Christian_K=F6nig?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Oded Gabbay , dri-devel@lists.freedesktop.org, Alexander Deucher CC: linux-kernel@vger.kernel.org, Huang Ying , LKP ML , ak@linux.intel.com, rusty@rustcorp.com.au, airlied@linux.ie Subject: Re: [PATCH v2] drm/radeon: Init amdkfd only if it was compiled References: <1419843586-29864-1-git-send-email-oded.gabbay@amd.com> In-Reply-To: <1419843586-29864-1-git-send-email-oded.gabbay@amd.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 29.12.2014 um 09:59 schrieb Oded Gabbay: > This patch changes the radeon_kfd_init(), which is used to initialize the > interface between radeon and amdkfd, so the interface will be initialized only > if amdkfd was build, either as module or inside the kernel image. > > In the modules case, the symbol_request() will be used (same as old code). In > the in-image compilation case, a direct call to kgd2kfd_init() will be done. > For other cases, radeon_kfd_init() will just return false. > > This patch is necessary because in case of the following specific > configuration: kernel 32-bit, no modules support, random kernel base and no > hibernation, the symbol_request() doesn't work as expected - it doesn't return > NULL if the symbol doesn't exists - which makes the kernel panic. > > Signed-off-by: Oded Gabbay Using symbol_request() in general doesn't seem to be such a good idea, because it breaks symbol versioning and as far as I can see is only very rarely used in drivers (AFAIK only V4L drivers). But for the moment I don't have another idea either except for always loading the module which on most hardware is completely unnecessary. Maybe we should consider restructuring the module load dependencies in the next kernel release. For now the patch is Reviewed-by: Christian König > --- > drivers/gpu/drm/radeon/radeon_kfd.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/radeon/radeon_kfd.c b/drivers/gpu/drm/radeon/radeon_kfd.c > index 242fd8b..d3e78b4 100644 > --- a/drivers/gpu/drm/radeon/radeon_kfd.c > +++ b/drivers/gpu/drm/radeon/radeon_kfd.c > @@ -101,6 +101,7 @@ static const struct kgd2kfd_calls *kgd2kfd; > > bool radeon_kfd_init(void) > { > +#if defined(CONFIG_HSA_AMD_MODULE) > bool (*kgd2kfd_init_p)(unsigned, const struct kfd2kgd_calls*, > const struct kgd2kfd_calls**); > > @@ -117,6 +118,17 @@ bool radeon_kfd_init(void) > } > > return true; > +#elif defined(CONFIG_HSA_AMD) > + if (!kgd2kfd_init(KFD_INTERFACE_VERSION, &kfd2kgd, &kgd2kfd)) { > + kgd2kfd = NULL; > + > + return false; > + } > + > + return true; > +#else > + return false; > +#endif > } > > void radeon_kfd_fini(void)