Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1250363 > unrolled thread

[PATCH] drm/sti: Remove select of CONFIG_FW_LOADER_USER_HELPER_FALLBACK

Started byTakashi Iwai <tiwai@suse.de>
First post2015-10-19 08:20 +0200
Last post2015-10-29 15:40 +0100
Articles 6 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] drm/sti: Remove select of CONFIG_FW_LOADER_USER_HELPER_FALLBACK Takashi Iwai <tiwai@suse.de> - 2015-10-19 08:20 +0200
    Re: [PATCH] drm/sti: Remove select of  CONFIG_FW_LOADER_USER_HELPER_FALLBACK Vincent ABRIOU <vincent.abriou@st.com> - 2015-10-29 15:30 +0100
      Re: [PATCH] drm/sti: Remove select of CONFIG_FW_LOADER_USER_HELPER_FALLBACK Emil Velikov <emil.l.velikov@gmail.com> - 2015-10-29 15:40 +0100
        Re: [PATCH] drm/sti: Remove select of CONFIG_FW_LOADER_USER_HELPER_FALLBACK Takashi Iwai <tiwai@suse.de> - 2015-10-29 16:00 +0100
          Re: [PATCH] drm/sti: Remove select of  CONFIG_FW_LOADER_USER_HELPER_FALLBACK Vincent ABRIOU <vincent.abriou@st.com> - 2015-11-02 10:20 +0100
      Re: [PATCH] drm/sti: Remove select of CONFIG_FW_LOADER_USER_HELPER_FALLBACK Takashi Iwai <tiwai@suse.de> - 2015-10-29 15:40 +0100

#1250363 — [PATCH] drm/sti: Remove select of CONFIG_FW_LOADER_USER_HELPER_FALLBACK

FromTakashi Iwai <tiwai@suse.de>
Date2015-10-19 08:20 +0200
Subject[PATCH] drm/sti: Remove select of CONFIG_FW_LOADER_USER_HELPER_FALLBACK
Message-ID<qlbXz-4l8-3@gated-at.bofh.it>
The commit [4fdbc678fe4d: drm: sti: add HQVDP plane] added the select
of CONFIG_FW_LOADER_USER_HELPER_FALLBACK by some unwritten reason.
But this config is known to be harmful, and is present only for
compatibility reason for an old exotic system that mandates udev
interaction which isn't supposed to be selected by a driver.
Let's remove it.

Fixes: 4fdbc678fe4d ('drm: sti: add HQVDP plane')
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/sti/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
index fbccc105819b..a18159074b76 100644
--- a/drivers/gpu/drm/sti/Kconfig
+++ b/drivers/gpu/drm/sti/Kconfig
@@ -6,7 +6,6 @@ config DRM_STI
 	select DRM_GEM_CMA_HELPER
 	select DRM_KMS_CMA_HELPER
 	select DRM_PANEL
-	select FW_LOADER_USER_HELPER_FALLBACK
 	help
 	  Choose this option to enable DRM on STM stiH41x chipset
 
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1258828 — Re: [PATCH] drm/sti: Remove select of CONFIG_FW_LOADER_USER_HELPER_FALLBACK

FromVincent ABRIOU <vincent.abriou@st.com>
Date2015-10-29 15:30 +0100
SubjectRe: [PATCH] drm/sti: Remove select of CONFIG_FW_LOADER_USER_HELPER_FALLBACK
Message-ID<qoWng-1hg-35@gated-at.bofh.it>
In reply to#1250363
Hi Takashi,

Removing FW_LOADER_USER_HELPER_FALLBACK leads to a failure in our HQVDP 
firmware execution.
Indeed, our firmware is not built-in. It is a proprietary firmware 
uploaded into the file system that's why we need the 
USER_HELPER_FALLBACK to be able to load it once file system is available.

BR
Vincent

On 10/19/2015 08:16 AM, Takashi Iwai wrote:
> The commit [4fdbc678fe4d: drm: sti: add HQVDP plane] added the select
> of CONFIG_FW_LOADER_USER_HELPER_FALLBACK by some unwritten reason.
> But this config is known to be harmful, and is present only for
> compatibility reason for an old exotic system that mandates udev
> interaction which isn't supposed to be selected by a driver.
> Let's remove it.
>
> Fixes: 4fdbc678fe4d ('drm: sti: add HQVDP plane')
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   drivers/gpu/drm/sti/Kconfig | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
> index fbccc105819b..a18159074b76 100644
> --- a/drivers/gpu/drm/sti/Kconfig
> +++ b/drivers/gpu/drm/sti/Kconfig
> @@ -6,7 +6,6 @@ config DRM_STI
>   	select DRM_GEM_CMA_HELPER
>   	select DRM_KMS_CMA_HELPER
>   	select DRM_PANEL
> -	select FW_LOADER_USER_HELPER_FALLBACK
>   	help
>   	  Choose this option to enable DRM on STM stiH41x chipset
>
>--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1258830

FromEmil Velikov <emil.l.velikov@gmail.com>
Date2015-10-29 15:40 +0100
Message-ID<qoWwV-1lC-5@gated-at.bofh.it>
In reply to#1258828
On 29 October 2015 at 14:21, Vincent ABRIOU <vincent.abriou@st.com> wrote:
> Hi Takashi,
>
> Removing FW_LOADER_USER_HELPER_FALLBACK leads to a failure in our HQVDP
> firmware execution.
> Indeed, our firmware is not built-in. It is a proprietary firmware
> uploaded into the file system that's why we need the
> USER_HELPER_FALLBACK to be able to load it once file system is available.
>
Hmm most other DRM drivers also require firmware. Whist some allow the
firmware to be picked in initrd it's not a strict requirement.
So I'm wondering how come there hasn't been (m)any reports,
considering that neither one sets USER_HELPER_FALLBACK.

Perhaps they also need it, or something in the sti module is done
differently ? Just some food for thought.

Regards,
Emil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1258848

FromTakashi Iwai <tiwai@suse.de>
Date2015-10-29 16:00 +0100
Message-ID<qoWQi-1vA-9@gated-at.bofh.it>
In reply to#1258830
On Thu, 29 Oct 2015 15:37:51 +0100,
Emil Velikov wrote:
> 
> On 29 October 2015 at 14:21, Vincent ABRIOU <vincent.abriou@st.com> wrote:
> > Hi Takashi,
> >
> > Removing FW_LOADER_USER_HELPER_FALLBACK leads to a failure in our HQVDP
> > firmware execution.
> > Indeed, our firmware is not built-in. It is a proprietary firmware
> > uploaded into the file system that's why we need the
> > USER_HELPER_FALLBACK to be able to load it once file system is available.
> >
> Hmm most other DRM drivers also require firmware. Whist some allow the
> firmware to be picked in initrd it's not a strict requirement.
> So I'm wondering how come there hasn't been (m)any reports,
> considering that neither one sets USER_HELPER_FALLBACK.
> 
> Perhaps they also need it, or something in the sti module is done
> differently ? Just some food for thought.

It's the option each user decides to set or not, depending on the
deployed system.  Most of PCs don't need them, and actually enabling
this option causes troubles for them.  On other embedded systems, this
might be still needed.  So, it's the system setup issue, and not the
thing a driver needs to care.

Imagine that your driver has "select EXT3_FS" because your system
requires it; without that option, it won't boot, OMG!
Is it the right thing?  Obviously no.  The same logic is applied to
this case, too.


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1260484 — Re: [PATCH] drm/sti: Remove select of CONFIG_FW_LOADER_USER_HELPER_FALLBACK

FromVincent ABRIOU <vincent.abriou@st.com>
Date2015-11-02 10:20 +0100
SubjectRe: [PATCH] drm/sti: Remove select of CONFIG_FW_LOADER_USER_HELPER_FALLBACK
Message-ID<qqjrs-3wz-17@gated-at.bofh.it>
In reply to#1258848
SGkgVGFrYXNoaSwNCg0KSSBtYWRlIGEgcmV3b3JrIHRvIGxvYWQgdGhlIGZpcm13YXJlIGF0IGEg
ZGlmZmVyZW50IHRpbWUsIHdoZW4gZmlsZSANCnN5c3RlbSBpcyBhdmFpbGFibGUuDQpJIGFtIG5v
dyBhYmxlIHRvIHJlbW92ZSBGV19MT0FERVJfVVNFUl9IRUxQRVJfRkFMTEJBQ0suDQpJIHdpbGwg
c3VibWl0IGEgbmV3IHBhdGNoIGZvciB0aGlzLg0KDQpWaW5jZW50DQoNCk9uIDEwLzI5LzIwMTUg
MDM6NTMgUE0sIFRha2FzaGkgSXdhaSB3cm90ZToNCj4gT24gVGh1LCAyOSBPY3QgMjAxNSAxNToz
Nzo1MSArMDEwMCwNCj4gRW1pbCBWZWxpa292IHdyb3RlOg0KPj4NCj4+IE9uIDI5IE9jdG9iZXIg
MjAxNSBhdCAxNDoyMSwgVmluY2VudCBBQlJJT1UgPHZpbmNlbnQuYWJyaW91QHN0LmNvbT4gd3Jv
dGU6DQo+Pj4gSGkgVGFrYXNoaSwNCj4+Pg0KPj4+IFJlbW92aW5nIEZXX0xPQURFUl9VU0VSX0hF
TFBFUl9GQUxMQkFDSyBsZWFkcyB0byBhIGZhaWx1cmUgaW4gb3VyIEhRVkRQDQo+Pj4gZmlybXdh
cmUgZXhlY3V0aW9uLg0KPj4+IEluZGVlZCwgb3VyIGZpcm13YXJlIGlzIG5vdCBidWlsdC1pbi4g
SXQgaXMgYSBwcm9wcmlldGFyeSBmaXJtd2FyZQ0KPj4+IHVwbG9hZGVkIGludG8gdGhlIGZpbGUg
c3lzdGVtIHRoYXQncyB3aHkgd2UgbmVlZCB0aGUNCj4+PiBVU0VSX0hFTFBFUl9GQUxMQkFDSyB0
byBiZSBhYmxlIHRvIGxvYWQgaXQgb25jZSBmaWxlIHN5c3RlbSBpcyBhdmFpbGFibGUuDQo+Pj4N
Cj4+IEhtbSBtb3N0IG90aGVyIERSTSBkcml2ZXJzIGFsc28gcmVxdWlyZSBmaXJtd2FyZS4gV2hp
c3Qgc29tZSBhbGxvdyB0aGUNCj4+IGZpcm13YXJlIHRvIGJlIHBpY2tlZCBpbiBpbml0cmQgaXQn
cyBub3QgYSBzdHJpY3QgcmVxdWlyZW1lbnQuDQo+PiBTbyBJJ20gd29uZGVyaW5nIGhvdyBjb21l
IHRoZXJlIGhhc24ndCBiZWVuIChtKWFueSByZXBvcnRzLA0KPj4gY29uc2lkZXJpbmcgdGhhdCBu
ZWl0aGVyIG9uZSBzZXRzIFVTRVJfSEVMUEVSX0ZBTExCQUNLLg0KPj4NCj4+IFBlcmhhcHMgdGhl
eSBhbHNvIG5lZWQgaXQsIG9yIHNvbWV0aGluZyBpbiB0aGUgc3RpIG1vZHVsZSBpcyBkb25lDQo+
PiBkaWZmZXJlbnRseSA/IEp1c3Qgc29tZSBmb29kIGZvciB0aG91Z2h0Lg0KPg0KPiBJdCdzIHRo
ZSBvcHRpb24gZWFjaCB1c2VyIGRlY2lkZXMgdG8gc2V0IG9yIG5vdCwgZGVwZW5kaW5nIG9uIHRo
ZQ0KPiBkZXBsb3llZCBzeXN0ZW0uICBNb3N0IG9mIFBDcyBkb24ndCBuZWVkIHRoZW0sIGFuZCBh
Y3R1YWxseSBlbmFibGluZw0KPiB0aGlzIG9wdGlvbiBjYXVzZXMgdHJvdWJsZXMgZm9yIHRoZW0u
ICBPbiBvdGhlciBlbWJlZGRlZCBzeXN0ZW1zLCB0aGlzDQo+IG1pZ2h0IGJlIHN0aWxsIG5lZWRl
ZC4gIFNvLCBpdCdzIHRoZSBzeXN0ZW0gc2V0dXAgaXNzdWUsIGFuZCBub3QgdGhlDQo+IHRoaW5n
IGEgZHJpdmVyIG5lZWRzIHRvIGNhcmUuDQo+DQo+IEltYWdpbmUgdGhhdCB5b3VyIGRyaXZlciBo
YXMgInNlbGVjdCBFWFQzX0ZTIiBiZWNhdXNlIHlvdXIgc3lzdGVtDQo+IHJlcXVpcmVzIGl0OyB3
aXRob3V0IHRoYXQgb3B0aW9uLCBpdCB3b24ndCBib290LCBPTUchDQo+IElzIGl0IHRoZSByaWdo
dCB0aGluZz8gIE9idmlvdXNseSBuby4gIFRoZSBzYW1lIGxvZ2ljIGlzIGFwcGxpZWQgdG8NCj4g
dGhpcyBjYXNlLCB0b28uDQo+DQo+DQo+IFRha2FzaGkNCj4gX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX18NCj4gZHJpLWRldmVsIG1haWxpbmcgbGlzdA0KPiBk
cmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnDQo+IGh0dHA6Ly9saXN0cy5mcmVlZGVza3Rv
cC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwNCj4=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1258832

FromTakashi Iwai <tiwai@suse.de>
Date2015-10-29 15:40 +0100
Message-ID<qoWwV-1lC-13@gated-at.bofh.it>
In reply to#1258828
On Thu, 29 Oct 2015 15:21:35 +0100,
Vincent ABRIOU wrote:
> 
> Hi Takashi,
> 
> Removing FW_LOADER_USER_HELPER_FALLBACK leads to a failure in our HQVDP 
> firmware execution.
> Indeed, our firmware is not built-in. It is a proprietary firmware 
> uploaded into the file system that's why we need the 
> USER_HELPER_FALLBACK to be able to load it once file system is available.

It's not the option a driver can set.  It's a global control option
that is applied *all* drivers.

If the fallback behavior is mandatory by a weird reason, you'd need to
adjust in a different way; e.g. modify the firmware loader code to
provide a new API for the driver to give a fallback (i.e. the another
flip of request_firmware_direct()) and use that API explicitly.

But I don't believe it's the case.  Judging from your comment, it's
your system setup that requires the user-space fallback.  So don't
make the fallback thing mandatory.


thanks,

Takashi

> 
> BR
> Vincent
> 
> On 10/19/2015 08:16 AM, Takashi Iwai wrote:
> > The commit [4fdbc678fe4d: drm: sti: add HQVDP plane] added the select
> > of CONFIG_FW_LOADER_USER_HELPER_FALLBACK by some unwritten reason.
> > But this config is known to be harmful, and is present only for
> > compatibility reason for an old exotic system that mandates udev
> > interaction which isn't supposed to be selected by a driver.
> > Let's remove it.
> >
> > Fixes: 4fdbc678fe4d ('drm: sti: add HQVDP plane')
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >   drivers/gpu/drm/sti/Kconfig | 1 -
> >   1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
> > index fbccc105819b..a18159074b76 100644
> > --- a/drivers/gpu/drm/sti/Kconfig
> > +++ b/drivers/gpu/drm/sti/Kconfig
> > @@ -6,7 +6,6 @@ config DRM_STI
> >   	select DRM_GEM_CMA_HELPER
> >   	select DRM_KMS_CMA_HELPER
> >   	select DRM_PANEL
> > -	select FW_LOADER_USER_HELPER_FALLBACK
> >   	help
> >   	  Choose this option to enable DRM on STM stiH41x chipset
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web