* [PATCH] drm: rcar-du: Fix R-Car Gen3 crash when VSP is disabled
@ 2016-10-26 3:31 Magnus Damm
2016-10-26 7:31 ` Geert Uytterhoeven
2016-10-26 7:33 ` Geert Uytterhoeven
0 siblings, 2 replies; 6+ messages in thread
From: Magnus Damm @ 2016-10-26 3:31 UTC (permalink / raw)
To: dri-devel
Cc: linux-renesas-soc, airlied, Magnus Damm, linux-kernel,
laurent.pinchart
From: Magnus Damm <damm+renesas@opensource.se>
For the DU to operate on R-Car Gen3 hardware a combination of DU
and VSP devices are required. Since the DU driver also supports
earlier generations hardware the VSP portion is enabled via Kconfig.
The arm64 defconfig is as of v4.9-rc1 having the DU driver enabled
as a module, however this is not enough to support R-Car Gen3. In
the current case of CONFIG_DRM_RCAR_VSP=n then the kernel crashes
when loading the module. This patch is fixing that particular case.
In more detail, the crash triggers in drm_atomic_get_plane_state()
when __drm_atomic_helper_set_config() passes NULL as crtc->primary.
This patch corrects this issue by failing to load the DU driver on
R-Car Gen3 when VSP is not available.
Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---
drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- 0001/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
+++ work/drivers/gpu/drm/rcar-du/rcar_du_vsp.h 2016-10-26 00:01:12.920607110 +0900
@@ -70,7 +70,7 @@ void rcar_du_vsp_disable(struct rcar_du_
void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc);
void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc);
#else
-static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp) { return 0; };
+static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp) { return -ENXIO; };
static inline void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) { };
static inline void rcar_du_vsp_disable(struct rcar_du_crtc *crtc) { };
static inline void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc) { };
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: rcar-du: Fix R-Car Gen3 crash when VSP is disabled
2016-10-26 3:31 [PATCH] drm: rcar-du: Fix R-Car Gen3 crash when VSP is disabled Magnus Damm
@ 2016-10-26 7:31 ` Geert Uytterhoeven
2016-10-26 9:13 ` Magnus Damm
2016-10-26 7:33 ` Geert Uytterhoeven
1 sibling, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2016-10-26 7:31 UTC (permalink / raw)
To: Magnus Damm
Cc: DRI Development, Linux-Renesas, David Airlie,
linux-kernel@vger.kernel.org, Laurent Pinchart
On Wed, Oct 26, 2016 at 5:31 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
>
> For the DU to operate on R-Car Gen3 hardware a combination of DU
> and VSP devices are required. Since the DU driver also supports
> earlier generations hardware the VSP portion is enabled via Kconfig.
>
> The arm64 defconfig is as of v4.9-rc1 having the DU driver enabled
> as a module, however this is not enough to support R-Car Gen3. In
> the current case of CONFIG_DRM_RCAR_VSP=n then the kernel crashes
> when loading the module. This patch is fixing that particular case.
>
> In more detail, the crash triggers in drm_atomic_get_plane_state()
> when __drm_atomic_helper_set_config() passes NULL as crtc->primary.
>
> This patch corrects this issue by failing to load the DU driver on
> R-Car Gen3 when VSP is not available.
>
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
>
> drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- 0001/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
> +++ work/drivers/gpu/drm/rcar-du/rcar_du_vsp.h 2016-10-26 00:01:12.920607110 +0900
> @@ -70,7 +70,7 @@ void rcar_du_vsp_disable(struct rcar_du_
> void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc);
> void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc);
> #else
> -static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp) { return 0; };
> +static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp) { return -ENXIO; };
-ENODEV sounds more appropriate
> static inline void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) { };
> static inline void rcar_du_vsp_disable(struct rcar_du_crtc *crtc) { };
> static inline void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc) { };
Alternatively, DRM_RCAR_DU can force DRM_RCAR_VSP to y if ARCH_R8A7795
or ARCH_R8A7796 is enabled.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: rcar-du: Fix R-Car Gen3 crash when VSP is disabled
2016-10-26 3:31 [PATCH] drm: rcar-du: Fix R-Car Gen3 crash when VSP is disabled Magnus Damm
2016-10-26 7:31 ` Geert Uytterhoeven
@ 2016-10-26 7:33 ` Geert Uytterhoeven
1 sibling, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2016-10-26 7:33 UTC (permalink / raw)
To: Magnus Damm
Cc: DRI Development, Linux-Renesas, David Airlie,
linux-kernel@vger.kernel.org, Laurent Pinchart
On Wed, Oct 26, 2016 at 5:31 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> The arm64 defconfig is as of v4.9-rc1 having the DU driver enabled
> as a module, however this is not enough to support R-Car Gen3. In
Nope, arm64 defconfig on v4.9-rc1:
# CONFIG_DRM_RCAR_DU is not set
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: rcar-du: Fix R-Car Gen3 crash when VSP is disabled
2016-10-26 7:31 ` Geert Uytterhoeven
@ 2016-10-26 9:13 ` Magnus Damm
2016-12-06 14:07 ` Laurent Pinchart
0 siblings, 1 reply; 6+ messages in thread
From: Magnus Damm @ 2016-10-26 9:13 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: DRI Development, Linux-Renesas, David Airlie,
linux-kernel@vger.kernel.org, Laurent Pinchart
Hi Geert,
On Wed, Oct 26, 2016 at 4:31 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Wed, Oct 26, 2016 at 5:31 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> From: Magnus Damm <damm+renesas@opensource.se>
>>
>> For the DU to operate on R-Car Gen3 hardware a combination of DU
>> and VSP devices are required. Since the DU driver also supports
>> earlier generations hardware the VSP portion is enabled via Kconfig.
>>
>> The arm64 defconfig is as of v4.9-rc1 having the DU driver enabled
>> as a module, however this is not enough to support R-Car Gen3. In
>> the current case of CONFIG_DRM_RCAR_VSP=n then the kernel crashes
>> when loading the module. This patch is fixing that particular case.
>>
>> In more detail, the crash triggers in drm_atomic_get_plane_state()
>> when __drm_atomic_helper_set_config() passes NULL as crtc->primary.
>>
>> This patch corrects this issue by failing to load the DU driver on
>> R-Car Gen3 when VSP is not available.
>>
>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>> ---
>>
>> drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> --- 0001/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
>> +++ work/drivers/gpu/drm/rcar-du/rcar_du_vsp.h 2016-10-26 00:01:12.920607110 +0900
>> @@ -70,7 +70,7 @@ void rcar_du_vsp_disable(struct rcar_du_
>> void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc);
>> void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc);
>> #else
>> -static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp) { return 0; };
>> +static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp) { return -ENXIO; };
>
> -ENODEV sounds more appropriate
Ok, however -ENXIO is the same error code as the DU driver currently
returns when dealing with the VSP and not finding DT nodes.
To avoid dealing with this again I would prefer skipping per-driver
Kconfig options entirely, for instance something like this:
[PATCH/RFC] Simplify Gen3 DU and VSP Kconfig bits
https://www.spinics.net/lists/linux-sh/msg45978.html
>> static inline void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) { };
>> static inline void rcar_du_vsp_disable(struct rcar_du_crtc *crtc) { };
>> static inline void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc) { };
>
> Alternatively, DRM_RCAR_DU can force DRM_RCAR_VSP to y if ARCH_R8A7795
> or ARCH_R8A7796 is enabled.
The DU driver has symbol dependencies on VSP and FCP driver code
(V4L2), so we need to take the state of driver support for those
modules into consideration as well instead of only depending on
SoC-specific Kconfig symbols.
On Wed, Oct 26, 2016 at 4:31 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>On Wed, Oct 26, 2016 at 5:31 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> The arm64 defconfig is as of v4.9-rc1 having the DU driver enabled
>> as a module, however this is not enough to support R-Car Gen3. In
>
> Nope, arm64 defconfig on v4.9-rc1:
> # CONFIG_DRM_RCAR_DU is not set
Right, you are correct. Sorry for the noise. It seems that I was using
renesas-drivers-2016-10-18-v4.9-rc1 - not mainline v4.9-rc1.
Without this fix the driver still crashes, but not due to defconfig in v4.9-rc1.
Thanks,
/ magnus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: rcar-du: Fix R-Car Gen3 crash when VSP is disabled
2016-10-26 9:13 ` Magnus Damm
@ 2016-12-06 14:07 ` Laurent Pinchart
2016-12-14 22:24 ` Laurent Pinchart
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2016-12-06 14:07 UTC (permalink / raw)
To: Magnus Damm
Cc: Geert Uytterhoeven, DRI Development, Linux-Renesas, David Airlie,
linux-kernel@vger.kernel.org
Hello,
On Wednesday 26 Oct 2016 18:13:23 Magnus Damm wrote:
> On Wed, Oct 26, 2016 at 4:31 PM, Geert Uytterhoeven wrote:
> > On Wed, Oct 26, 2016 at 5:31 AM, Magnus Damm wrote:
> >> From: Magnus Damm <damm+renesas@opensource.se>
> >>
> >> For the DU to operate on R-Car Gen3 hardware a combination of DU
> >> and VSP devices are required. Since the DU driver also supports
> >> earlier generations hardware the VSP portion is enabled via Kconfig.
> >>
> >> The arm64 defconfig is as of v4.9-rc1 having the DU driver enabled
> >> as a module, however this is not enough to support R-Car Gen3. In
> >> the current case of CONFIG_DRM_RCAR_VSP=n then the kernel crashes
> >> when loading the module. This patch is fixing that particular case.
> >>
> >> In more detail, the crash triggers in drm_atomic_get_plane_state()
> >> when __drm_atomic_helper_set_config() passes NULL as crtc->primary.
> >>
> >> This patch corrects this issue by failing to load the DU driver on
> >> R-Car Gen3 when VSP is not available.
> >>
> >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> >> ---
> >>
> >> drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> --- 0001/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
> >> +++ work/drivers/gpu/drm/rcar-du/rcar_du_vsp.h 2016-10-26
> >> 00:01:12.920607110 +0900 @@ -70,7 +70,7 @@ void
> >> rcar_du_vsp_disable(struct rcar_du_
> >>
> >> void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc);
> >> void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc);
> >> #else
> >>
> >> -static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp) { return 0;
> >> };
> >> +static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp) { return
> >> -ENXIO; };
With this patch applied the DU will fail to probe on Gen2 if DRM_RCAR_VSP is
disabled.
DRM_RCAR_DU should instead depend on VIDEO_RENESAS_VSP1 (with DRM_RCAR_VSP
always set) on Gen3.
> > -ENODEV sounds more appropriate
>
> Ok, however -ENXIO is the same error code as the DU driver currently
> returns when dealing with the VSP and not finding DT nodes.
>
> To avoid dealing with this again I would prefer skipping per-driver
> Kconfig options entirely, for instance something like this:
>
> [PATCH/RFC] Simplify Gen3 DU and VSP Kconfig bits
> https://www.spinics.net/lists/linux-sh/msg45978.html
>
> >> static inline void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) { };
> >> static inline void rcar_du_vsp_disable(struct rcar_du_crtc *crtc) { };
> >> static inline void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc) {
> >> };
> >
> > Alternatively, DRM_RCAR_DU can force DRM_RCAR_VSP to y if ARCH_R8A7795
> > or ARCH_R8A7796 is enabled.
>
> The DU driver has symbol dependencies on VSP and FCP driver code
> (V4L2), so we need to take the state of driver support for those
> modules into consideration as well instead of only depending on
> SoC-specific Kconfig symbols.
>
> On Wed, Oct 26, 2016 at 4:31 PM, Geert Uytterhoeven wrote:
> > On Wed, Oct 26, 2016 at 5:31 AM, Magnus Damm wrote:
> >> The arm64 defconfig is as of v4.9-rc1 having the DU driver enabled
> >> as a module, however this is not enough to support R-Car Gen3. In
> >
> > Nope, arm64 defconfig on v4.9-rc1:
> > # CONFIG_DRM_RCAR_DU is not set
>
> Right, you are correct. Sorry for the noise. It seems that I was using
> renesas-drivers-2016-10-18-v4.9-rc1 - not mainline v4.9-rc1.
>
> Without this fix the driver still crashes, but not due to defconfig in
> v4.9-rc1.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: rcar-du: Fix R-Car Gen3 crash when VSP is disabled
2016-12-06 14:07 ` Laurent Pinchart
@ 2016-12-14 22:24 ` Laurent Pinchart
0 siblings, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2016-12-14 22:24 UTC (permalink / raw)
To: Magnus Damm
Cc: Geert Uytterhoeven, DRI Development, Linux-Renesas, David Airlie,
linux-kernel@vger.kernel.org
Hi Magnus,
On Tuesday 06 Dec 2016 16:07:02 Laurent Pinchart wrote:
> On Wednesday 26 Oct 2016 18:13:23 Magnus Damm wrote:
> > On Wed, Oct 26, 2016 at 4:31 PM, Geert Uytterhoeven wrote:
> >> On Wed, Oct 26, 2016 at 5:31 AM, Magnus Damm wrote:
> >>> From: Magnus Damm <damm+renesas@opensource.se>
> >>>
> >>> For the DU to operate on R-Car Gen3 hardware a combination of DU
> >>> and VSP devices are required. Since the DU driver also supports
> >>> earlier generations hardware the VSP portion is enabled via Kconfig.
> >>>
> >>> The arm64 defconfig is as of v4.9-rc1 having the DU driver enabled
> >>> as a module, however this is not enough to support R-Car Gen3. In
> >>> the current case of CONFIG_DRM_RCAR_VSP=n then the kernel crashes
> >>> when loading the module. This patch is fixing that particular case.
> >>>
> >>> In more detail, the crash triggers in drm_atomic_get_plane_state()
> >>> when __drm_atomic_helper_set_config() passes NULL as crtc->primary.
> >>>
> >>> This patch corrects this issue by failing to load the DU driver on
> >>> R-Car Gen3 when VSP is not available.
> >>>
> >>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> >>> ---
> >>>
> >>> drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> --- 0001/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
> >>> +++ work/drivers/gpu/drm/rcar-du/rcar_du_vsp.h 2016-10-26
> >>> 00:01:12.920607110 +0900 @@ -70,7 +70,7 @@ void
> >>> rcar_du_vsp_disable(struct rcar_du_
> >>>
> >>> void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc);
> >>> void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc);
> >>> #else
> >>>
> >>> -static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp) { return
> >>> 0;
> >>> };
> >>> +static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp) { return
> >>> -ENXIO; };
>
> With this patch applied the DU will fail to probe on Gen2 if DRM_RCAR_VSP is
> disabled.
>
> DRM_RCAR_DU should instead depend on VIDEO_RENESAS_VSP1 (with DRM_RCAR_VSP
> always set) on Gen3.
Do you plan to work on a v2 that fixes this in Kconfig ? I gave it a quick try
but ran into a Kconfig circular dependency that I don't have time to debug
now.
> > > -ENODEV sounds more appropriate
> >
> > Ok, however -ENXIO is the same error code as the DU driver currently
> > returns when dealing with the VSP and not finding DT nodes.
> >
> > To avoid dealing with this again I would prefer skipping per-driver
> > Kconfig options entirely, for instance something like this:
> >
> > [PATCH/RFC] Simplify Gen3 DU and VSP Kconfig bits
> > https://www.spinics.net/lists/linux-sh/msg45978.html
> >
> >>> static inline void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) { };
> >>> static inline void rcar_du_vsp_disable(struct rcar_du_crtc *crtc) { };
> >>> static inline void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc)
> >>> {
> >>> };
> >>
> >> Alternatively, DRM_RCAR_DU can force DRM_RCAR_VSP to y if ARCH_R8A7795
> >> or ARCH_R8A7796 is enabled.
> >
> > The DU driver has symbol dependencies on VSP and FCP driver code
> > (V4L2), so we need to take the state of driver support for those
> > modules into consideration as well instead of only depending on
> > SoC-specific Kconfig symbols.
> >
> > On Wed, Oct 26, 2016 at 4:31 PM, Geert Uytterhoeven wrote:
> >> On Wed, Oct 26, 2016 at 5:31 AM, Magnus Damm wrote:
> >>> The arm64 defconfig is as of v4.9-rc1 having the DU driver enabled
> >>> as a module, however this is not enough to support R-Car Gen3. In
> >>
> >> Nope, arm64 defconfig on v4.9-rc1:
> >> # CONFIG_DRM_RCAR_DU is not set
> >
> > Right, you are correct. Sorry for the noise. It seems that I was using
> > renesas-drivers-2016-10-18-v4.9-rc1 - not mainline v4.9-rc1.
> >
> > Without this fix the driver still crashes, but not due to defconfig in
> > v4.9-rc1.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-14 22:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-26 3:31 [PATCH] drm: rcar-du: Fix R-Car Gen3 crash when VSP is disabled Magnus Damm
2016-10-26 7:31 ` Geert Uytterhoeven
2016-10-26 9:13 ` Magnus Damm
2016-12-06 14:07 ` Laurent Pinchart
2016-12-14 22:24 ` Laurent Pinchart
2016-10-26 7:33 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox