Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH V2] video: exynos_dp: Fix incorrect setting for INT_CTL
From: Jingoo Han @ 2012-11-05  7:38 UTC (permalink / raw)
  To: linux-fbdev

On Monday, November 05, 2012 4:43 PM Ajay Kumar wrote
> 
> INT_CTL register contains bits INT_POL0 and INT_POL1, and not INT_POL.
> This patch fixes the wrong register setting for INT_CTL.
> 
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> ---
>  drivers/video/exynos/exynos_dp_reg.c |    2 +-
>  drivers/video/exynos/exynos_dp_reg.h |    3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
> index 3f5ca8a..d67f49b 100644
> --- a/drivers/video/exynos/exynos_dp_reg.c
> +++ b/drivers/video/exynos/exynos_dp_reg.c
> @@ -88,7 +88,7 @@ void exynos_dp_init_analog_param(struct exynos_dp_device *dp)
>  void exynos_dp_init_interrupt(struct exynos_dp_device *dp)
>  {
>  	/* Set interrupt pin assertion polarity as high */
> -	writel(INT_POL, dp->reg_base + EXYNOS_DP_INT_CTL);
> +	writel(INT_POL0 | INT_POL1, dp->reg_base + EXYNOS_DP_INT_CTL);
> 
>  	/* Clear pending regisers */
>  	writel(0xff, dp->reg_base + EXYNOS_DP_COMMON_INT_STA_1);
> diff --git a/drivers/video/exynos/exynos_dp_reg.h b/drivers/video/exynos/exynos_dp_reg.h
> index 1f2f014..8548b91 100644
> --- a/drivers/video/exynos/exynos_dp_reg.h
> +++ b/drivers/video/exynos/exynos_dp_reg.h
> @@ -242,7 +242,8 @@
> 
>  /* EXYNOS_DP_INT_CTL */
>  #define SOFT_INT_CTRL				(0x1 << 2)
> -#define INT_POL					(0x1 << 0)
> +#define INT_POL0				(0x1 << 0)
> +#define INT_POL1				(0x1 << 1)

Please keep the bit order in descending order, for readability.
It is not big deal, so I will send the v3 patch, soon.


> 
>  /* EXYNOS_DP_SYS_CTL_1 */
>  #define DET_STA					(0x1 << 2)
> --
> 1.7.0.4


^ permalink raw reply

* [PATCH v3 1/2] video: exynos_dp: Reset and initialize DP before requesting irq
From: Jingoo Han @ 2012-11-05  7:43 UTC (permalink / raw)
  To: linux-fbdev

If DP is not reset properly before kernel bootup(in bootloader code),
there can be few pending interrupts, and sometimes they invoke
DP irq handler as soon as the irq handler is registered in DP probe.
So, we make the DP driver more robust by resetting and
initializing DP at the earliest and then registering the irq handler.

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
Tested with Exynos5250

 drivers/video/exynos/exynos_dp_core.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index d55470e..d241fd3 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -901,6 +901,12 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	dp->video_info = pdata->video_info;
+	if (pdata->phy_init)
+		pdata->phy_init();
+
+	exynos_dp_init_dp(dp);
+
 	ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
 				"exynos-dp", dp);
 	if (ret) {
@@ -908,12 +914,6 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	dp->video_info = pdata->video_info;
-	if (pdata->phy_init)
-		pdata->phy_init();
-
-	exynos_dp_init_dp(dp);
-
 	ret = exynos_dp_detect_hpd(dp);
 	if (ret) {
 		dev_err(&pdev->dev, "unable to detect hpd\n");
-- 
1.7.1



^ permalink raw reply related

* [PATCH v3 2/2] video: exynos_dp: Fix incorrect setting for INT_CTL
From: Jingoo Han @ 2012-11-05  7:44 UTC (permalink / raw)
  To: linux-fbdev

INT_CTL register contains bits INT_POL0 and INT_POL1, and not INT_POL.
This patch fixes the wrong register setting for INT_CTL.

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
Tested with Exynos5250

 drivers/video/exynos/exynos_dp_reg.c |    2 +-
 drivers/video/exynos/exynos_dp_reg.h |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 3f5ca8a..cc7765f 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -88,7 +88,7 @@ void exynos_dp_init_analog_param(struct exynos_dp_device *dp)
 void exynos_dp_init_interrupt(struct exynos_dp_device *dp)
 {
 	/* Set interrupt pin assertion polarity as high */
-	writel(INT_POL, dp->reg_base + EXYNOS_DP_INT_CTL);
+	writel(INT_POL1 | INT_POL0, dp->reg_base + EXYNOS_DP_INT_CTL);
 
 	/* Clear pending regisers */
 	writel(0xff, dp->reg_base + EXYNOS_DP_COMMON_INT_STA_1);
diff --git a/drivers/video/exynos/exynos_dp_reg.h b/drivers/video/exynos/exynos_dp_reg.h
index 1f2f014..2e9bd0e 100644
--- a/drivers/video/exynos/exynos_dp_reg.h
+++ b/drivers/video/exynos/exynos_dp_reg.h
@@ -242,7 +242,8 @@
 
 /* EXYNOS_DP_INT_CTL */
 #define SOFT_INT_CTRL				(0x1 << 2)
-#define INT_POL					(0x1 << 0)
+#define INT_POL1				(0x1 << 1)
+#define INT_POL0				(0x1 << 0)
 
 /* EXYNOS_DP_SYS_CTL_1 */
 #define DET_STA					(0x1 << 2)
-- 
1.7.1



^ permalink raw reply related

* [PATCH v3 1/2] video: exynos_dp: Reset and initialize DP before requesting irq
From: Jingoo Han @ 2012-11-05  7:51 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <003301cdbb29$421e2ee0$c65a8ca0$%han@samsung.com>

From: Ajay Kumar <ajaykumar.rs@samsung.com>

If DP is not reset properly before kernel bootup(in bootloader code),
there can be few pending interrupts, and sometimes they invoke
DP irq handler as soon as the irq handler is registered in DP probe.
So, we make the DP driver more robust by resetting and
initializing DP at the earliest and then registering the irq handler.

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
Added 'From: Ajay Kumar <ajaykumar.rs@samsung.com>'

 drivers/video/exynos/exynos_dp_core.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index d55470e..d241fd3 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -901,6 +901,12 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	dp->video_info = pdata->video_info;
+	if (pdata->phy_init)
+		pdata->phy_init();
+
+	exynos_dp_init_dp(dp);
+
 	ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
 				"exynos-dp", dp);
 	if (ret) {
@@ -908,12 +914,6 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	dp->video_info = pdata->video_info;
-	if (pdata->phy_init)
-		pdata->phy_init();
-
-	exynos_dp_init_dp(dp);
-
 	ret = exynos_dp_detect_hpd(dp);
 	if (ret) {
 		dev_err(&pdev->dev, "unable to detect hpd\n");
-- 
1.7.1




^ permalink raw reply related

* [PATCH v3 2/2] video: exynos_dp: Fix incorrect setting for INT_CTL
From: Jingoo Han @ 2012-11-05  7:52 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <003401cdbb29$60b46220$221d2660$%han@samsung.com>

From: Ajay Kumar <ajaykumar.rs@samsung.com>

INT_CTL register contains bits INT_POL0 and INT_POL1, and not INT_POL.
This patch fixes the wrong register setting for INT_CTL.

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
Added 'From: Ajay Kumar <ajaykumar.rs@samsung.com>'

 drivers/video/exynos/exynos_dp_reg.c |    2 +-
 drivers/video/exynos/exynos_dp_reg.h |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 3f5ca8a..cc7765f 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -88,7 +88,7 @@ void exynos_dp_init_analog_param(struct exynos_dp_device *dp)
 void exynos_dp_init_interrupt(struct exynos_dp_device *dp)
 {
 	/* Set interrupt pin assertion polarity as high */
-	writel(INT_POL, dp->reg_base + EXYNOS_DP_INT_CTL);
+	writel(INT_POL1 | INT_POL0, dp->reg_base + EXYNOS_DP_INT_CTL);
 
 	/* Clear pending regisers */
 	writel(0xff, dp->reg_base + EXYNOS_DP_COMMON_INT_STA_1);
diff --git a/drivers/video/exynos/exynos_dp_reg.h b/drivers/video/exynos/exynos_dp_reg.h
index 1f2f014..2e9bd0e 100644
--- a/drivers/video/exynos/exynos_dp_reg.h
+++ b/drivers/video/exynos/exynos_dp_reg.h
@@ -242,7 +242,8 @@
 
 /* EXYNOS_DP_INT_CTL */
 #define SOFT_INT_CTRL				(0x1 << 2)
-#define INT_POL					(0x1 << 0)
+#define INT_POL1				(0x1 << 1)
+#define INT_POL0				(0x1 << 0)
 
 /* EXYNOS_DP_SYS_CTL_1 */
 #define DET_STA					(0x1 << 2)
-- 
1.7.1



^ permalink raw reply related

* [PATCH V2] video: exynos_dp: Reset and initialize DP before requesting irq
From: Ajay Kumar @ 2012-11-05  7:54 UTC (permalink / raw)
  To: linux-fbdev

If DP is not reset properly before kernel bootup(in bootloader code),
there can be few pending interrupts, and sometimes they invoke
DP irq handler as soon as the irq handler is registered in DP probe.
So, we make the DP driver more robust by resetting and
initializing DP at the earliest and then registering the irq handler.

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
Acked-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/exynos/exynos_dp_core.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index d55470e..d241fd3 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -901,6 +901,12 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	dp->video_info = pdata->video_info;
+	if (pdata->phy_init)
+		pdata->phy_init();
+
+	exynos_dp_init_dp(dp);
+
 	ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
 				"exynos-dp", dp);
 	if (ret) {
@@ -908,12 +914,6 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	dp->video_info = pdata->video_info;
-	if (pdata->phy_init)
-		pdata->phy_init();
-
-	exynos_dp_init_dp(dp);
-
 	ret = exynos_dp_detect_hpd(dp);
 	if (ret) {
 		dev_err(&pdev->dev, "unable to detect hpd\n");
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH V2] video: exynos_dp: Fix incorrect setting for INT_CTL
From: Ajay Kumar @ 2012-11-05  7:54 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <002901cdbb28$82ecb7d0$88c62770$%han@samsung.com>

INT_CTL register contains bits INT_POL0 and INT_POL1, and not INT_POL.
This patch fixes the wrong register setting for INT_CTL.

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
---
 drivers/video/exynos/exynos_dp_reg.c |    2 +-
 drivers/video/exynos/exynos_dp_reg.h |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 3f5ca8a..d67f49b 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -88,7 +88,7 @@ void exynos_dp_init_analog_param(struct exynos_dp_device *dp)
 void exynos_dp_init_interrupt(struct exynos_dp_device *dp)
 {
 	/* Set interrupt pin assertion polarity as high */
-	writel(INT_POL, dp->reg_base + EXYNOS_DP_INT_CTL);
+	writel(INT_POL0 | INT_POL1, dp->reg_base + EXYNOS_DP_INT_CTL);
 
 	/* Clear pending regisers */
 	writel(0xff, dp->reg_base + EXYNOS_DP_COMMON_INT_STA_1);
diff --git a/drivers/video/exynos/exynos_dp_reg.h b/drivers/video/exynos/exynos_dp_reg.h
index 1f2f014..8548b91 100644
--- a/drivers/video/exynos/exynos_dp_reg.h
+++ b/drivers/video/exynos/exynos_dp_reg.h
@@ -242,7 +242,8 @@
 
 /* EXYNOS_DP_INT_CTL */
 #define SOFT_INT_CTRL				(0x1 << 2)
-#define INT_POL					(0x1 << 0)
+#define INT_POL0				(0x1 << 0)
+#define INT_POL1				(0x1 << 1)
 
 /* EXYNOS_DP_SYS_CTL_1 */
 #define DET_STA					(0x1 << 2)
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
From: Tomi Valkeinen @ 2012-11-05  8:55 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <5093B4F3.1000703@ti.com>

[-- Attachment #1: Type: text/plain, Size: 3267 bytes --]

On 2012-11-02 13:56, Archit Taneja wrote:
> On Friday 02 November 2012 04:58 PM, Tomi Valkeinen wrote:
>> On 2012-11-02 13:09, Archit Taneja wrote:
>>> On Friday 02 November 2012 04:19 PM, Tomi Valkeinen wrote:
>>>> On 2012-11-02 12:44, Archit Taneja wrote:
>>>>
>>>>> Hmm, that makes sense. Anyway, I don't think it's really bad if we
>>>>> refer
>>>>> to dssdev->channel for now.
>>>>
>>>> It is, because dssdev->channel doesn't exist with DT.
>>>>
>>>> With DT we either need to figure out the channel in omapdss at runtime,
>>>> or add a property to the DT data telling the channel. And adding such a
>>>> property is not correct, as DT should be about describing the HW.
>>>
>>> Ok.
>>>
>>> I don't totally agree with your idea of figuring out the manager in
>>> panel the panel's probe. If it's done in the panel driver's probe
>>> itself, then by this point of time we have already set
>>> mgr->output->device links. If omapdss only does this stuff, then
>>
>> Hmm, I'm not sure I understand what's your point above? If figuring out
>> the mgr is done in panel's probe, the mgr->output link is not yet made
>> before that time.
> 
> My point is that we are trying to find a manager at panel's probe
> itself. It think that's what we do now. But one of your recent patch
> moves that to omapfb.

Ah. Yes, that's true.

>>> omapfb/omapdrm have just the job of connecting the overlays to the
>>> manager. Do you think that's okay?
>>
>> Yes, that's how I think it should be. I don't see why omapfb/omapdrm
>> should care about which manager is being used for the output, it doesn't
>> really matter as long there is one and it works.
>>
>> Then again, I don't have anything against omapfb/omapdrm choosing the
>> manager, but I don't see how they would have any better idea of which
>> manager to use than omapdss.
>>
>> But as doing the connections at probe time is a bit problematic, perhaps
>> we should have a new step in this whole sequence. Something like
>> "connect" or whatever, which would lock the required blocks in the whole
>> pipeline, and acquire the required resources that couldn't be gotten at
>> probe time.
>>
>> But even then, choosing the manager is not easy, as whoever chooses the
>> manager needs to observe all the possible displays used at the same
>> time...
> 
> Right. I was wondering if omapfb/omapdrm could understand the 'all
> possible displays information' better compared to a panel's probe.
> 
> Even omapdrm/omafb can't be perfect because we could insert a panel
> driver module at any time, and omapfb/omapdrm may miss that out.

True, omapdrm/fb may have a better idea. It's still unclear though.
Currently we have quite strict order in the sequence the modules need to
be loaded, which is quite bad and causes issues. We should make things
more dynamic, so that the initialization of the drivers could happen
more freely.

But that creates more problems: when booting up, omapfb starts. But
omapfb can't know if all the panel drivers have already been loaded.
omapfb may see that DVI is the default display, but what should it do if
DVI doesn't have a driver yet? It could wait, but perhaps the driver for
DVI will never even be loaded.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

^ permalink raw reply

* Re: [PATCH v7 8/8] drm_modes: add of_videomode helpers
From: Thierry Reding @ 2012-11-05  9:10 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Guennady Liakhovetski, linux-media,
	Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-9-git-send-email-s.trumtrar@pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 787 bytes --]

On Wed, Oct 31, 2012 at 10:28:08AM +0100, Steffen Trumtrar wrote:
[...]
> +/**
> + * of_get_drm_display_mode - get a drm_display_mode from devicetree
> + * @np: device_node with the timing specification
> + * @dmode: will be set to the return value
> + * @index: index into the list of display timings in devicetree
> + * 
> + * DESCRIPTION:

I don't think this is necessary.

> + * This function is expensive and should only be used, if only one mode is to be
> + * read from DT. To get multiple modes start with of_get_display_timing_list ond

You probably meant "and" at the end of this line. Also I'm not even sure
that we should be exposing this function, but rather provide a helper
which automatically adds the parsed modes to a DRM connector object.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [GIT PULL] omapdss fixes for 3.7-rc
From: Tomi Valkeinen @ 2012-11-05  9:52 UTC (permalink / raw)
  To: Florian Tobias Schandinat, linux-omap@vger.kernel.org,
	linux-fbdev

[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]

Hi Florian,

Here are a few omapdss fixes, two fixing crasher bugs and the third
fixes a missing unlock. These are based on 3.7-rc1. I can rebase on
top of something else if you prefer that.

 Tomi


The following changes since commit ddffeb8c4d0331609ef2581d84de4d763607bd37:

  Linux 3.7-rc1 (2012-10-14 14:41:04 -0700)

are available in the git repository at:

  git://gitorious.org/linux-omap-dss2/linux.git tags/omapdss-for-3.7-rc5

for you to fetch changes up to dffc70ade1d13edd186f542718c4d78a31d92fb8:

  OMAPDSS: HDMI: fix missing unlock on error in hdmi_dump_regs() (2012-10-26 08:45:43 +0300)

----------------------------------------------------------------
omapdss fixes for 3.7-rc5

omapdss fixes for two crashes bugs and one missing mutex unlock.

----------------------------------------------------------------
Laurent Pinchart (1):
      omapdss: dss: Fix clocks on OMAP363x

Tomi Valkeinen (1):
      OMAPDSS: DSI: fix dsi_get_dsidev_from_id()

Wei Yongjun (1):
      OMAPDSS: HDMI: fix missing unlock on error in hdmi_dump_regs()

 drivers/video/omap2/dss/dsi.c  |   13 +++++++++++--
 drivers/video/omap2/dss/dss.c  |    4 ++--
 drivers/video/omap2/dss/hdmi.c |    4 +++-
 3 files changed, 16 insertions(+), 5 deletions(-)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

^ permalink raw reply

* Only for test purpose
From: Stefan Koch @ 2012-11-05 13:00 UTC (permalink / raw)
  To: linux-fbdev

This is a test mail. Because I cannot see my mails to this mailing list 
in marc.info archive, but messages from other people that have send 
their messages later I can see.

^ permalink raw reply

* Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
From: Rob Clark @ 2012-11-05 14:21 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Archit Taneja, linux-omap, linux-fbdev, dri-devel
In-Reply-To: <50977EEF.3040403@ti.com>

On 11/05/2012 02:55 AM, Tomi Valkeinen wrote:
>>> But even then, choosing the manager is not easy, as whoever chooses the
>>> >>manager needs to observe all the possible displays used at the same
>>> >>time...
>> >
>> >Right. I was wondering if omapfb/omapdrm could understand the 'all
>> >possible displays information' better compared to a panel's probe.
>> >
>> >Even omapdrm/omafb can't be perfect because we could insert a panel
>> >driver module at any time, and omapfb/omapdrm may miss that out.
> True, omapdrm/fb may have a better idea. It's still unclear though.
> Currently we have quite strict order in the sequence the modules need to
> be loaded, which is quite bad and causes issues. We should make things
> more dynamic, so that the initialization of the drivers could happen
> more freely.
>
> But that creates more problems: when booting up, omapfb starts. But
> omapfb can't know if all the panel drivers have already been loaded.
> omapfb may see that DVI is the default display, but what should it do if
> DVI doesn't have a driver yet? It could wait, but perhaps the driver for
> DVI will never even be loaded.

The encoder which is connected to the crtc (manager) is picked by 
combination of encoder->possible_crtcs bitmask and 
connector->best_encoder().  We could keep things limited so that the 
association of crtc to encoder (manager to output, roughly) never 
changes, but this isn't really the right thing to do.  It is better that 
the dssdev not rely on knowing the manager it is attached to at probe 
time, but instead grab resources more dynamically.

Also, at the moment we don't really have any notification to userspace 
about new encoders/connectors showing up (or conversely, being 
removed).  Only about existing connectors being plugged/unplugged.  The 
closest analogy is perhaps the USB display devices, but even there it is 
only the entire drm device that is plugged/unplugged.  And TBH I don't 
really see the point in supporting panel drivers being dynamically 
loaded.  It isn't like someone is dynamically soldering on a new display 
connector to some board that is running.  I think omapfb or omapdrm 
probe should trigger registering the compiled-in panel drivers, so that 
it can be sure that the dssdev's pop up before it goes and creates drm 
connector objects.  Currently we have to hack around this in omapdrm 
with late_initcall() to ensure the panel drivers are probed first, but 
that is an ugly hack that I'd like to get rid of.

BR,
-R




^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Sasha Levin @ 2012-11-05 17:26 UTC (permalink / raw)
  To: Alan Cox
  Cc: Sasha Levin, Greg Kroah-Hartman, Jiri Slaby,
	linux-kernel@vger.kernel.org, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <20121026143754.50277bd8@pyramind.ukuu.org.uk>

Ping? Should I bisect it?

On Fri, Oct 26, 2012 at 9:37 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Thu, 25 Oct 2012 15:37:43 -0400
> Sasha Levin <sasha.levin@oracle.com> wrote:
>
>> Hi all,
>>
>> While fuzzing with trinity inside a KVM tools (lkvm) guest running latest -next kernel,
>> I've stumbled on the following spew:
>
> Looks real enough but its not a tty/vt layer spew. This is all coming out
> of the core framebuffer code which doesn't seem to be able to decide what
> the locking rules at the invocation of fb_notifier_call_chain are.
>
> It might need some console layer tweaking to provide 'register console
> and I already hold the locks' or similar but that notifier needs some
> kind of sanity applying as well.
>
> Cc'ing the fbdev folks
>
> Alan
> --
> 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/

^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Alan Cox @ 2012-11-05 17:59 UTC (permalink / raw)
  To: Sasha Levin
  Cc: Sasha Levin, Greg Kroah-Hartman, Jiri Slaby,
	linux-kernel@vger.kernel.org, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <CA+1xoqdEesjh1EZvR_r7Hn0GUc7741EDWn59qagngcxg4=9bjQ@mail.gmail.com>

On Mon, 5 Nov 2012 12:26:43 -0500
Sasha Levin <levinsasha928@gmail.com> wrote:

> Ping? Should I bisect it?
> 
> On Fri, Oct 26, 2012 at 9:37 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > On Thu, 25 Oct 2012 15:37:43 -0400
> > Sasha Levin <sasha.levin@oracle.com> wrote:
> >
> >> Hi all,
> >>
> >> While fuzzing with trinity inside a KVM tools (lkvm) guest running latest -next kernel,
> >> I've stumbled on the following spew:
> >
> > Looks real enough but its not a tty/vt layer spew. This is all coming out
> > of the core framebuffer code which doesn't seem to be able to decide what
> > the locking rules at the invocation of fb_notifier_call_chain are.
> >
> > It might need some console layer tweaking to provide 'register console
> > and I already hold the locks' or similar but that notifier needs some
> > kind of sanity applying as well.
> >
> > Cc'ing the fbdev folks

I've cc'd the framebuffer folks. I can see why its occurring but I have
no idea how they intend to fix it and I've not seen any replies.

Sorry but I've got enough other things on my plate right now without
trying to deal with the locking brain damage that the fbdev layer is.

As far as I can tell the actual bug proper is years old.

Alan


^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Sasha Levin @ 2012-11-05 18:00 UTC (permalink / raw)
  To: Alan Cox
  Cc: Sasha Levin, Greg Kroah-Hartman, Jiri Slaby,
	linux-kernel@vger.kernel.org, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <20121105175937.26f31d2a@pyramind.ukuu.org.uk>

On 11/05/2012 12:59 PM, Alan Cox wrote:
> On Mon, 5 Nov 2012 12:26:43 -0500
> Sasha Levin <levinsasha928@gmail.com> wrote:
> 
>> Ping? Should I bisect it?
>>
>> On Fri, Oct 26, 2012 at 9:37 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>>> On Thu, 25 Oct 2012 15:37:43 -0400
>>> Sasha Levin <sasha.levin@oracle.com> wrote:
>>>
>>>> Hi all,
>>>>
>>>> While fuzzing with trinity inside a KVM tools (lkvm) guest running latest -next kernel,
>>>> I've stumbled on the following spew:
>>>
>>> Looks real enough but its not a tty/vt layer spew. This is all coming out
>>> of the core framebuffer code which doesn't seem to be able to decide what
>>> the locking rules at the invocation of fb_notifier_call_chain are.
>>>
>>> It might need some console layer tweaking to provide 'register console
>>> and I already hold the locks' or similar but that notifier needs some
>>> kind of sanity applying as well.
>>>
>>> Cc'ing the fbdev folks
> 
> I've cc'd the framebuffer folks. I can see why its occurring but I have
> no idea how they intend to fix it and I've not seen any replies.
> 
> Sorry but I've got enough other things on my plate right now without
> trying to deal with the locking brain damage that the fbdev layer is.
> 
> As far as I can tell the actual bug proper is years old.
> 
> Alan
> 

Ow, I figured it's something new since I've only now started seeing it in fuzz
tests, and it reproduces pretty much every time.


Thanks,
Sasha

^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Hugh Dickins @ 2012-11-05 19:17 UTC (permalink / raw)
  To: Sasha Levin
  Cc: Daniel Vetter, Alan Cox, Sasha Levin, Greg Kroah-Hartman,
	Jiri Slaby, linux-kernel, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <5097FEA9.2090603@oracle.com>

On Mon, 5 Nov 2012, Sasha Levin wrote:
> On 11/05/2012 12:59 PM, Alan Cox wrote:
> > On Mon, 5 Nov 2012 12:26:43 -0500
> > Sasha Levin <levinsasha928@gmail.com> wrote:
> > 
> >> Ping? Should I bisect it?
> >>
> >> On Fri, Oct 26, 2012 at 9:37 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> >>> On Thu, 25 Oct 2012 15:37:43 -0400
> >>> Sasha Levin <sasha.levin@oracle.com> wrote:
> >>>
> >>>> Hi all,
> >>>>
> >>>> While fuzzing with trinity inside a KVM tools (lkvm) guest running latest -next kernel,
> >>>> I've stumbled on the following spew:
> >>>
> >>> Looks real enough but its not a tty/vt layer spew. This is all coming out
> >>> of the core framebuffer code which doesn't seem to be able to decide what
> >>> the locking rules at the invocation of fb_notifier_call_chain are.
> >>>
> >>> It might need some console layer tweaking to provide 'register console
> >>> and I already hold the locks' or similar but that notifier needs some
> >>> kind of sanity applying as well.
> >>>
> >>> Cc'ing the fbdev folks
> > 
> > I've cc'd the framebuffer folks. I can see why its occurring but I have
> > no idea how they intend to fix it and I've not seen any replies.
> > 
> > Sorry but I've got enough other things on my plate right now without
> > trying to deal with the locking brain damage that the fbdev layer is.
> > 
> > As far as I can tell the actual bug proper is years old.
> > 
> > Alan
> > 
> 
> Ow, I figured it's something new since I've only now started seeing it in fuzz
> tests, and it reproduces pretty much every time.

The fbdev potential for deadlock may be years old, but the warning
(and consequent disabling of lockdep from that point on - making it
useless to everybody else in need of it) is new, and comes from the
commit below in linux-next.

I revert it in my own testing: if there is no quick fix to the
fbdev issue on the way, Daniel, please revert it from your tree.

Thanks,
Hugh

commit daee779718a319ff9f83e1ba3339334ac650bb22
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Sat Sep 22 19:52:11 2012 +0200

    console: implement lockdep support for console_lock
    
    Dave Airlie recently discovered a locking bug in the fbcon layer,
    where a timer_del_sync (for the blinking cursor) deadlocks with the
    timer itself, since both (want to) hold the console_lock:
    
    https://lkml.org/lkml/2012/8/21/36
    
    Unfortunately the console_lock isn't a plain mutex and hence has no
    lockdep support. Which resulted in a few days wasted of tracking down
    this bug (complicated by the fact that printk doesn't show anything
    when the console is locked) instead of noticing the bug much earlier
    with the lockdep splat.
    
    Hence I've figured I need to fix that for the next deadlock involving
    console_lock - and with kms/drm growing ever more complex locking
    that'll eventually happen.
    
    Now the console_lock has rather funky semantics, so after a quick irc
    discussion with Thomas Gleixner and Dave Airlie I've quickly ditched
    the original idead of switching to a real mutex (since it won't work)
    and instead opted to annotate the console_lock with lockdep
    information manually.
    
    There are a few special cases:
    - The console_lock state is protected by the console_sem, and usually
      grabbed/dropped at _lock/_unlock time. But the suspend/resume code
      drops the semaphore without dropping the console_lock (see
      suspend_console/resume_console). But since the same thread that did
      the suspend will do the resume, we don't need to fix up anything.
    
    - In the printk code there's a special trylock, only used to kick off
      the logbuffer printk'ing in console_unlock. But all that happens
      while lockdep is disable (since printk does a few other evil
      tricks). So no issue there, either.
    
    - The console_lock can also be acquired form irq context (but only
      with a trylock). lockdep already handles that.
    
    This all leaves us with annotating the normal console_lock, _unlock
    and _trylock functions.
    
    And yes, it works - simply unloading a drm kms driver resulted in
    lockdep complaining about the deadlock in fbcon_deinit:
    
    ===========================
    [ INFO: possible circular locking dependency detected ]
    3.6.0-rc2+ #552 Not tainted
    -------------------------------------------------------
    kms-reload/3577 is trying to acquire lock:
     ((&info->queue)){+.+...}, at: [<ffffffff81058c70>] wait_on_work+0x0/0xa7
    
    but task is already holding lock:
     (console_lock){+.+.+.}, at: [<ffffffff81264686>] bind_con_driver+0x38/0x263
    
    which lock already depends on the new lock.
    
    the existing dependency chain (in reverse order) is:
    
    -> #1 (console_lock){+.+.+.}:
           [<ffffffff81087440>] lock_acquire+0x95/0x105
           [<ffffffff81040190>] console_lock+0x59/0x5b
           [<ffffffff81209cb6>] fb_flashcursor+0x2e/0x12c
           [<ffffffff81057c3e>] process_one_work+0x1d9/0x3b4
           [<ffffffff810584a2>] worker_thread+0x1a7/0x24b
           [<ffffffff8105ca29>] kthread+0x7f/0x87
           [<ffffffff813b1204>] kernel_thread_helper+0x4/0x10
    
    -> #0 ((&info->queue)){+.+...}:
           [<ffffffff81086cb3>] __lock_acquire+0x999/0xcf6
           [<ffffffff81087440>] lock_acquire+0x95/0x105
           [<ffffffff81058cab>] wait_on_work+0x3b/0xa7
           [<ffffffff81058dd6>] __cancel_work_timer+0xbf/0x102
           [<ffffffff81058e33>] cancel_work_sync+0xb/0xd
           [<ffffffff8120a3b3>] fbcon_deinit+0x11c/0x1dc
           [<ffffffff81264793>] bind_con_driver+0x145/0x263
           [<ffffffff81264a45>] unbind_con_driver+0x14f/0x195
           [<ffffffff8126540c>] store_bind+0x1ad/0x1c1
           [<ffffffff8127cbb7>] dev_attr_store+0x13/0x1f
           [<ffffffff8116d884>] sysfs_write_file+0xe9/0x121
           [<ffffffff811145b2>] vfs_write+0x9b/0xfd
           [<ffffffff811147b7>] sys_write+0x3e/0x6b
           [<ffffffff813b0039>] system_call_fastpath+0x16/0x1b
    
    other info that might help us debug this:
    
     Possible unsafe locking scenario:
    
           CPU0                    CPU1
           ----                    ----
      lock(console_lock);
                                   lock((&info->queue));
                                   lock(console_lock);
      lock((&info->queue));
    
     *** DEADLOCK ***
    
    v2: Mark the lockdep_map static, noticed by Jani Nikula.
    
    Cc: Dave Airlie <airlied@gmail.com>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
    Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/kernel/printk.c b/kernel/printk.c
index 2d607f4..ee79f14 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -87,6 +87,12 @@ static DEFINE_SEMAPHORE(console_sem);
 struct console *console_drivers;
 EXPORT_SYMBOL_GPL(console_drivers);
 
+#ifdef CONFIG_LOCKDEP
+static struct lockdep_map console_lock_dep_map = {
+	.name = "console_lock"
+};
+#endif
+
 /*
  * This is used for debugging the mess that is the VT code by
  * keeping track if we have the console semaphore held. It's
@@ -1914,6 +1920,7 @@ void console_lock(void)
 		return;
 	console_locked = 1;
 	console_may_schedule = 1;
+	mutex_acquire(&console_lock_dep_map, 0, 0, _RET_IP_);
 }
 EXPORT_SYMBOL(console_lock);
 
@@ -1935,6 +1942,7 @@ int console_trylock(void)
 	}
 	console_locked = 1;
 	console_may_schedule = 0;
+	mutex_acquire(&console_lock_dep_map, 0, 1, _RET_IP_);
 	return 1;
 }
 EXPORT_SYMBOL(console_trylock);
@@ -2095,6 +2103,7 @@ skip:
 		local_irq_restore(flags);
 	}
 	console_locked = 0;
+	mutex_release(&console_lock_dep_map, 1, _RET_IP_);
 
 	/* Release the exclusive_console once it is used */
 	if (unlikely(exclusive_console))


^ permalink raw reply related

* Re: tty, vt: lockdep warnings
From: Alan Cox @ 2012-11-05 20:15 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Sasha Levin, Daniel Vetter, Sasha Levin, Greg Kroah-Hartman,
	Jiri Slaby, linux-kernel, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <alpine.LNX.2.00.1211051106590.1709@eggly.anvils>

> The fbdev potential for deadlock may be years old, but the warning
> (and consequent disabling of lockdep from that point on - making it
> useless to everybody else in need of it) is new, and comes from the
> commit below in linux-next.
> 
> I revert it in my own testing: if there is no quick fix to the
> fbdev issue on the way, Daniel, please revert it from your tree.

If you revert it you swap it for a different deadlock - and one that
happens more often I would expect. Not very useful.

I'm hoping the framebuffer maintainer will bother to respond to this
because that's the only way it can be sorted out.

Alan

^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Hugh Dickins @ 2012-11-05 20:34 UTC (permalink / raw)
  To: Alan Cox
  Cc: Sasha Levin, Daniel Vetter, Sasha Levin, Greg Kroah-Hartman,
	Jiri Slaby, linux-kernel, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <20121105201507.79fe47d7@pyramind.ukuu.org.uk>

On Mon, 5 Nov 2012, Alan Cox wrote:
> > The fbdev potential for deadlock may be years old, but the warning
> > (and consequent disabling of lockdep from that point on - making it
> > useless to everybody else in need of it) is new, and comes from the
> > commit below in linux-next.
> > 
> > I revert it in my own testing: if there is no quick fix to the
> > fbdev issue on the way, Daniel, please revert it from your tree.
> 
> If you revert it you swap it for a different deadlock - and one that
> happens more often I would expect. Not very useful.

But a deadlock we have lived with for years.  Without reverting,
we're prevented from discovering all the new deadlocks we're adding.

> 
> I'm hoping the framebuffer maintainer will bother to respond to this
> because that's the only way it can be sorted out.

That would be ideal - thanks.

Hugh

^ permalink raw reply

* Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
From: Tomi Valkeinen @ 2012-11-06 13:41 UTC (permalink / raw)
  To: Rob Clark
  Cc: Tomi Valkeinen, Archit Taneja, linux-omap, linux-fbdev, dri-devel
In-Reply-To: <5097CB81.7050204@ti.com>

On 2012-11-05 16:21, Rob Clark wrote:
> On 11/05/2012 02:55 AM, Tomi Valkeinen wrote:
>>>> But even then, choosing the manager is not easy, as whoever chooses the
>>>> >>manager needs to observe all the possible displays used at the same
>>>> >>time...
>>> >
>>> >Right. I was wondering if omapfb/omapdrm could understand the 'all
>>> >possible displays information' better compared to a panel's probe.
>>> >
>>> >Even omapdrm/omafb can't be perfect because we could insert a panel
>>> >driver module at any time, and omapfb/omapdrm may miss that out.
>> True, omapdrm/fb may have a better idea. It's still unclear though.
>> Currently we have quite strict order in the sequence the modules need to
>> be loaded, which is quite bad and causes issues. We should make things
>> more dynamic, so that the initialization of the drivers could happen
>> more freely.
>>
>> But that creates more problems: when booting up, omapfb starts. But
>> omapfb can't know if all the panel drivers have already been loaded.
>> omapfb may see that DVI is the default display, but what should it do if
>> DVI doesn't have a driver yet? It could wait, but perhaps the driver for
>> DVI will never even be loaded.
> 
> The encoder which is connected to the crtc (manager) is picked by
> combination of encoder->possible_crtcs bitmask and
> connector->best_encoder().  We could keep things limited so that the
> association of crtc to encoder (manager to output, roughly) never
> changes, but this isn't really the right thing to do.  It is better that
> the dssdev not rely on knowing the manager it is attached to at probe
> time, but instead grab resources more dynamically.
> 
> Also, at the moment we don't really have any notification to userspace
> about new encoders/connectors showing up (or conversely, being
> removed).  Only about existing connectors being plugged/unplugged.  The
> closest analogy is perhaps the USB display devices, but even there it is
> only the entire drm device that is plugged/unplugged.  And TBH I don't
> really see the point in supporting panel drivers being dynamically
> loaded.  It isn't like someone is dynamically soldering on a new display
> connector to some board that is running.  I think omapfb or omapdrm
> probe should trigger registering the compiled-in panel drivers, so that
> it can be sure that the dssdev's pop up before it goes and creates drm
> connector objects.  Currently we have to hack around this in omapdrm
> with late_initcall() to ensure the panel drivers are probed first, but
> that is an ugly hack that I'd like to get rid of.

We have panel devices and panel drivers, each of which can appear at any
time. Both are needed for the panel probe to happen. If we don't support
device hotplugging (dynamic creation of devices), we need to use
late_initcall for omapfb/drm. At least I don't see any other option.

You say that omapdrm should trigger registering of the drivers. How
would that work? Do you mean that the panel drivers would register
themselves to some common list, and omapdrm would go through this list
when drm is loaded, calling probe for the items in the list? I guess
that's doable, but... It's not how kernel drivers are supposed to work,
and so doesn't sound very clean approach to me.

I think we should support proper hotplugging of the panels. This would
fix the problem about init order, but it would also give us device
hotplug support. Obviously nobody is going to solder panel to a running
board, but I don't see any reason why panels, or, more likely, panels on
an add-on boards (like the capes being discussed in omap ml) would not
be hotpluggable using whatever connector is used on the particular use case.

And even if we don't support removing of the devices, things like the
add-on capes could cause the panel on the cape to be identified at some
late time (the panel is not described in the board file or DT data, but
found at runtime depending on the ID of the cape). This would add
another step to the init sequence that should be just right, if we don't
support hotplug.

Yes, I know it's not simple =). And I'm fine with simpler approach for
the time being, but I'd like full hotplug to be the future goal. At
least the common panel framework should not create restrictions about
this, even if drm wouldn't allow device hotplug.

 Tomi


^ permalink raw reply

* Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
From: Rob Clark @ 2012-11-06 14:40 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Tomi Valkeinen, Archit Taneja, linux-omap, linux-fbdev, dri-devel
In-Reply-To: <509913A4.5080408@iki.fi>

On Tue, Nov 6, 2012 at 7:41 AM, Tomi Valkeinen <tomba@iki.fi> wrote:
> On 2012-11-05 16:21, Rob Clark wrote:
>> On 11/05/2012 02:55 AM, Tomi Valkeinen wrote:
>>>>> But even then, choosing the manager is not easy, as whoever chooses the
>>>>> >>manager needs to observe all the possible displays used at the same
>>>>> >>time...
>>>> >
>>>> >Right. I was wondering if omapfb/omapdrm could understand the 'all
>>>> >possible displays information' better compared to a panel's probe.
>>>> >
>>>> >Even omapdrm/omafb can't be perfect because we could insert a panel
>>>> >driver module at any time, and omapfb/omapdrm may miss that out.
>>> True, omapdrm/fb may have a better idea. It's still unclear though.
>>> Currently we have quite strict order in the sequence the modules need to
>>> be loaded, which is quite bad and causes issues. We should make things
>>> more dynamic, so that the initialization of the drivers could happen
>>> more freely.
>>>
>>> But that creates more problems: when booting up, omapfb starts. But
>>> omapfb can't know if all the panel drivers have already been loaded.
>>> omapfb may see that DVI is the default display, but what should it do if
>>> DVI doesn't have a driver yet? It could wait, but perhaps the driver for
>>> DVI will never even be loaded.
>>
>> The encoder which is connected to the crtc (manager) is picked by
>> combination of encoder->possible_crtcs bitmask and
>> connector->best_encoder().  We could keep things limited so that the
>> association of crtc to encoder (manager to output, roughly) never
>> changes, but this isn't really the right thing to do.  It is better that
>> the dssdev not rely on knowing the manager it is attached to at probe
>> time, but instead grab resources more dynamically.
>>
>> Also, at the moment we don't really have any notification to userspace
>> about new encoders/connectors showing up (or conversely, being
>> removed).  Only about existing connectors being plugged/unplugged.  The
>> closest analogy is perhaps the USB display devices, but even there it is
>> only the entire drm device that is plugged/unplugged.  And TBH I don't
>> really see the point in supporting panel drivers being dynamically
>> loaded.  It isn't like someone is dynamically soldering on a new display
>> connector to some board that is running.  I think omapfb or omapdrm
>> probe should trigger registering the compiled-in panel drivers, so that
>> it can be sure that the dssdev's pop up before it goes and creates drm
>> connector objects.  Currently we have to hack around this in omapdrm
>> with late_initcall() to ensure the panel drivers are probed first, but
>> that is an ugly hack that I'd like to get rid of.
>
> We have panel devices and panel drivers, each of which can appear at any
> time. Both are needed for the panel probe to happen. If we don't support
> device hotplugging (dynamic creation of devices), we need to use
> late_initcall for omapfb/drm. At least I don't see any other option.
>
> You say that omapdrm should trigger registering of the drivers. How
> would that work? Do you mean that the panel drivers would register
> themselves to some common list, and omapdrm would go through this list
> when drm is loaded, calling probe for the items in the list? I guess
> that's doable, but... It's not how kernel drivers are supposed to work,
> and so doesn't sound very clean approach to me.

I mean, similar to how we handle the subdev for dmm..  the
omap_drm_init() does the platform_driver_register() for the dmm device
before the platform_driver_register() for omapdrm itself, so we know
if there is a dmm device, the driver gets probed first before omapdrm.

It could be a matter of iterating through a list, or something like
this.. that is basically an implementation detail.  But the end result
is that the order the drivers are registered is controlled so the
probe sequence works out properly (not to mention suspend/resume
sequence).

> I think we should support proper hotplugging of the panels. This would
> fix the problem about init order, but it would also give us device
> hotplug support. Obviously nobody is going to solder panel to a running
> board, but I don't see any reason why panels, or, more likely, panels on
> an add-on boards (like the capes being discussed in omap ml) would not
> be hotpluggable using whatever connector is used on the particular use case.
>
> And even if we don't support removing of the devices, things like the
> add-on capes could cause the panel on the cape to be identified at some
> late time (the panel is not described in the board file or DT data, but
> found at runtime depending on the ID of the cape). This would add
> another step to the init sequence that should be just right, if we don't
> support hotplug.

If capes are really hot-pluggable, then maybe it is worth thinking
about how to make this more dynamic.  Although it is a bigger problem,
which involves userspace being aware that connectors can dynamically
appear/disappear.  And the dynamic disappearing is something I worry
about more.. it adds the possibility of all sorts of interesting race
conditions, such as connectors disappearing in the middle of modeset.
I prefer not making things more complicated and error prone than they
need to be.  If there is not a legitimate use case for connector hw
dynamically appearing/disappearing then I don't think we should go
there.  It sounds nice and simple and clean, but in reality I think it
just introduces a whole lot of ways for things to go wrong.  A wise
man once said:

https://github.com/robclark/kernel-omap4/blob/master/Documentation/SubmittingPatches#L700

BR,
-R

> Yes, I know it's not simple =). And I'm fine with simpler approach for
> the time being, but I'd like full hotplug to be the future goal. At
> least the common panel framework should not create restrictions about
> this, even if drm wouldn't allow device hotplug.
>
>  Tomi
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Alan Cox @ 2012-11-06 16:11 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Sasha Levin, Daniel Vetter, Sasha Levin, Greg Kroah-Hartman,
	Jiri Slaby, linux-kernel, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <alpine.LNX.2.00.1211051231070.21645@eggly.anvils>

On Mon, 5 Nov 2012 12:34:44 -0800 (PST)
Hugh Dickins <hughd@google.com> wrote:

> On Mon, 5 Nov 2012, Alan Cox wrote:
> > > The fbdev potential for deadlock may be years old, but the warning
> > > (and consequent disabling of lockdep from that point on - making it
> > > useless to everybody else in need of it) is new, and comes from the
> > > commit below in linux-next.
> > > 
> > > I revert it in my own testing: if there is no quick fix to the
> > > fbdev issue on the way, Daniel, please revert it from your tree.
> > 
> > If you revert it you swap it for a different deadlock - and one that
> > happens more often I would expect. Not very useful.
> 
> But a deadlock we have lived with for years.  Without reverting,
> we're prevented from discovering all the new deadlocks we're adding.

We lived with it locking boxes up on users but not knowing why. The root
cause is loading two different framebuffers with one taking over from
another - that should be an obscure corner case and once the fuzz testing
can avoid.
 
> That would be ideal - thanks.


I had a semi-informed poke at this and came up with a possible patch (not very tested)


commit f4fa6c739ecc367dbb98f5be1ff626d9b2750878
Author: Alan Cox <alan@linux.intel.com>
Date:   Tue Nov 6 15:33:18 2012 +0000

    fb: Rework locking to fix lock ordering on takeover
    
    Adjust the console layer to allow a take over call where the caller already
    holds the locks. Make the fb layer lock in order.
    
    This s partly a band aid, the fb layer is terminally confused about the
    locking rules it uses for its notifiers it seems.
    
    Signed-off-by: Alan Cox <alan@linux.intel.com>

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index f87d7e8..ea57f27 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -2984,7 +2984,7 @@ int __init vty_init(const struct file_operations *console_fops)
 
 static struct class *vtconsole_class;
 
-static int bind_con_driver(const struct consw *csw, int first, int last,
+static int do_bind_con_driver(const struct consw *csw, int first, int last,
 			   int deflt)
 {
 	struct module *owner = csw->owner;
@@ -2995,7 +2995,7 @@ static int bind_con_driver(const struct consw *csw, int first, int last,
 	if (!try_module_get(owner))
 		return -ENODEV;
 
-	console_lock();
+	WARN_CONSOLE_UNLOCKED();
 
 	/* check if driver is registered */
 	for (i = 0; i < MAX_NR_CON_DRIVER; i++) {
@@ -3080,11 +3080,22 @@ static int bind_con_driver(const struct consw *csw, int first, int last,
 
 	retval = 0;
 err:
-	console_unlock();
 	module_put(owner);
 	return retval;
 };
 
+
+static int bind_con_driver(const struct consw *csw, int first, int last,
+			   int deflt)
+{
+	int ret;
+	
+	console_unlock();
+	ret = do_bind_con_driver(csw, first, last, deflt);
+	console_unlock();
+	return ret;
+}
+	
 #ifdef CONFIG_VT_HW_CONSOLE_BINDING
 static int con_is_graphics(const struct consw *csw, int first, int last)
 {
@@ -3196,9 +3207,9 @@ int unbind_con_driver(const struct consw *csw, int first, int last, int deflt)
 	if (!con_is_bound(csw))
 		con_driver->flag &= ~CON_DRIVER_FLAG_INIT;
 
-	console_unlock();
 	/* ignore return value, binding should not fail */
-	bind_con_driver(defcsw, first, last, deflt);
+	do_bind_con_driver(defcsw, first, last, deflt);
+	console_unlock();
 err:
 	module_put(owner);
 	return retval;
@@ -3489,28 +3500,18 @@ int con_debug_leave(void)
 }
 EXPORT_SYMBOL_GPL(con_debug_leave);
 
-/**
- * register_con_driver - register console driver to console layer
- * @csw: console driver
- * @first: the first console to take over, minimum value is 0
- * @last: the last console to take over, maximum value is MAX_NR_CONSOLES -1
- *
- * DESCRIPTION: This function registers a console driver which can later
- * bind to a range of consoles specified by @first and @last. It will
- * also initialize the console driver by calling con_startup().
- */
-int register_con_driver(const struct consw *csw, int first, int last)
+static int do_register_con_driver(const struct consw *csw, int first, int last)
 {
 	struct module *owner = csw->owner;
 	struct con_driver *con_driver;
 	const char *desc;
 	int i, retval = 0;
 
+	WARN_CONSOLE_UNLOCKED();
+
 	if (!try_module_get(owner))
 		return -ENODEV;
 
-	console_lock();
-
 	for (i = 0; i < MAX_NR_CON_DRIVER; i++) {
 		con_driver = &registered_con_driver[i];
 
@@ -3563,10 +3564,29 @@ int register_con_driver(const struct consw *csw, int first, int last)
 	}
 
 err:
-	console_unlock();
 	module_put(owner);
 	return retval;
 }
+
+/**
+ * register_con_driver - register console driver to console layer
+ * @csw: console driver
+ * @first: the first console to take over, minimum value is 0
+ * @last: the last console to take over, maximum value is MAX_NR_CONSOLES -1
+ *
+ * DESCRIPTION: This function registers a console driver which can later
+ * bind to a range of consoles specified by @first and @last. It will
+ * also initialize the console driver by calling con_startup().
+ */
+int register_con_driver(const struct consw *csw, int first, int last)
+{
+	int retval;
+	
+	console_lock();
+	retval = do_register_con_driver(csw, first, last);
+	console_unlock();
+	return retval;
+}
 EXPORT_SYMBOL(register_con_driver);
 
 /**
@@ -3622,6 +3642,29 @@ EXPORT_SYMBOL(unregister_con_driver);
  *
  *      take_over_console is basically a register followed by unbind
  */
+int do_take_over_console(const struct consw *csw, int first, int last, int deflt)
+{
+	int err;
+
+	err = do_register_con_driver(csw, first, last);
+	/* if we get an busy error we still want to bind the console driver
+	 * and return success, as we may have unbound the console driver
+	 * but not unregistered it.
+	*/
+	if (err = -EBUSY)
+		err = 0;
+	if (!err)
+		do_bind_con_driver(csw, first, last, deflt);
+
+	return err;
+}
+/*
+ *	If we support more console drivers, this function is used
+ *	when a driver wants to take over some existing consoles
+ *	and become default driver for newly opened ones.
+ *
+ *      take_over_console is basically a register followed by unbind
+ */
 int take_over_console(const struct consw *csw, int first, int last, int deflt)
 {
 	int err;
diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index fdefa8f..c75f8ce 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -529,6 +529,34 @@ static int search_for_mapped_con(void)
 	return retval;
 }
 
+static int do_fbcon_takeover(int show_logo)
+{
+	int err, i;
+
+	if (!num_registered_fb)
+		return -ENODEV;
+
+	if (!show_logo)
+		logo_shown = FBCON_LOGO_DONTSHOW;
+
+	for (i = first_fb_vc; i <= last_fb_vc; i++)
+		con2fb_map[i] = info_idx;
+
+	err = do_take_over_console(&fb_con, first_fb_vc, last_fb_vc,
+				fbcon_is_default);
+
+	if (err) {
+		for (i = first_fb_vc; i <= last_fb_vc; i++) {
+			con2fb_map[i] = -1;
+		}
+		info_idx = -1;
+	} else {
+		fbcon_has_console_bind = 1;
+	}
+
+	return err;
+}
+
 static int fbcon_takeover(int show_logo)
 {
 	int err, i;
@@ -3115,7 +3143,7 @@ static int fbcon_fb_registered(struct fb_info *info)
 		}
 
 		if (info_idx != -1)
-			ret = fbcon_takeover(1);
+			ret = do_fbcon_takeover(1);
 	} else {
 		for (i = first_fb_vc; i <= last_fb_vc; i++) {
 			if (con2fb_map_boot[i] = idx)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 3ff0105..588bdab 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1650,7 +1650,9 @@ static int do_register_framebuffer(struct fb_info *fb_info)
 	event.info = fb_info;
 	if (!lock_fb_info(fb_info))
 		return -ENODEV;
+        console_lock();
 	fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
+        console_unlock();
 	unlock_fb_info(fb_info);
 	return 0;
 }
diff --git a/include/linux/console.h b/include/linux/console.h
index dedb082..4ef4307 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -78,6 +78,7 @@ int con_is_bound(const struct consw *csw);
 int register_con_driver(const struct consw *csw, int first, int last);
 int unregister_con_driver(const struct consw *csw);
 int take_over_console(const struct consw *sw, int first, int last, int deflt);
+int do_take_over_console(const struct consw *sw, int first, int last, int deflt);
 void give_up_console(const struct consw *sw);
 #ifdef CONFIG_HW_CONSOLE
 int con_debug_enter(struct vc_data *vc);

^ permalink raw reply related

* Re: tty, vt: lockdep warnings
From: Dave Jones @ 2012-11-06 16:42 UTC (permalink / raw)
  To: Alan Cox
  Cc: Hugh Dickins, Sasha Levin, Daniel Vetter, Sasha Levin,
	Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-fbdev,
	florianSchandinat
In-Reply-To: <20121106161100.216c6d79@pyramind.ukuu.org.uk>

On Tue, Nov 06, 2012 at 04:11:00PM +0000, Alan Cox wrote:

 > > But a deadlock we have lived with for years.  Without reverting,
 > > we're prevented from discovering all the new deadlocks we're adding.
 > 
 > We lived with it locking boxes up on users but not knowing why.

Circa 3.5 we got a lot more reports of this happening too for some reason.
Turns out that races are awfully resistant to bisecting too.

 > The root
 > cause is loading two different framebuffers with one taking over from
 > another - that should be an obscure corner case and once the fuzz testing
 > can avoid.
 > 
 > I had a semi-informed poke at this and came up with a possible patch (not very tested)

If this fixes the real problems we've been seeing, I'll dance a jig.
 
	Dave


^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Alan Cox @ 2012-11-06 17:38 UTC (permalink / raw)
  To: Dave Jones
  Cc: Hugh Dickins, Sasha Levin, Daniel Vetter, Sasha Levin,
	Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-fbdev,
	florianSchandinat
In-Reply-To: <20121106164214.GA18246@redhat.com>

>  > The root
>  > cause is loading two different framebuffers with one taking over from
>  > another - that should be an obscure corner case and once the fuzz testing
>  > can avoid.
>  > 
>  > I had a semi-informed poke at this and came up with a possible patch (not very tested)
> 
> If this fixes the real problems we've been seeing, I'll dance a jig.

Youtube...

At this point my bigger concern is that it'll just make something else
warn instead. The underlying problem is that fbcon layer implements a
single threaded notifier whose locking semantics are at best random. It's
not calld with a specific set of locks each time. Possibly it sohuld be
two notifiers (one for fb stuff, one for console layer stuff) but the
entire layer is horrible. I live in home the KMS guys will rip out the
useful bits and build a straight kms fb layer with refcounting and the
like 8)

Testing certainly needed and if it's still blowing up then hopefully
further traces will help fix up the other cases we don't know about.

Alan

^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Hugh Dickins @ 2012-11-07  4:29 UTC (permalink / raw)
  To: Alan Cox
  Cc: Sasha Levin, Daniel Vetter, Sasha Levin, Greg Kroah-Hartman,
	Jiri Slaby, linux-kernel, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <20121106161100.216c6d79@pyramind.ukuu.org.uk>

On Tue, 6 Nov 2012, Alan Cox wrote:
> On Mon, 5 Nov 2012 12:34:44 -0800 (PST)
> Hugh Dickins <hughd@google.com> wrote:
> > On Mon, 5 Nov 2012, Alan Cox wrote:
> > > > The fbdev potential for deadlock may be years old, but the warning
> > > > (and consequent disabling of lockdep from that point on - making it
> > > > useless to everybody else in need of it) is new, and comes from the
> > > > commit below in linux-next.
> > > > 
> > > > I revert it in my own testing: if there is no quick fix to the
> > > > fbdev issue on the way, Daniel, please revert it from your tree.
> > > 
> > > If you revert it you swap it for a different deadlock - and one that
> > > happens more often I would expect. Not very useful.
> > 
> > But a deadlock we have lived with for years.  Without reverting,
> > we're prevented from discovering all the new deadlocks we're adding.
> 
> We lived with it locking boxes up on users but not knowing why. The root
> cause is loading two different framebuffers with one taking over from
> another - that should be an obscure corner case and once the fuzz testing
> can avoid.

I'm bemused, but at least I now understand why we disagreed on this.

You thought it was a lockdep splat I got in the course of fuzz testing,
or doing some other obscure test: no, I thought I got it in booting up
the laptop, so it was in the way of doing useful testing thereafter.

I'd swear that I saw it two or three times, on each boot of 3.7.0-rc3-mm1;
then lost patience and deleted all the console_lock_dep_map lines from
kernel/printk.c, after which no problem.

But /var/log/messages calls me a liar, shows only one instance, and that
10 minutes after booting: that splat appended below in case it tells you
anything new; but I've no idea what triggered iti.  (The "W" taint comes
from my using a "numaúke=2" boot option, which surprised smpboot.c to
find smt-siblings on different nodes: not related to the console, I hope).

>  
> > That would be ideal - thanks.
> 
> 
> I had a semi-informed poke at this and came up with a possible patch (not very tested)

Many thanks for your effort.

> 
> commit f4fa6c739ecc367dbb98f5be1ff626d9b2750878
> Author: Alan Cox <alan@linux.intel.com>
> Date:   Tue Nov 6 15:33:18 2012 +0000
> 
>     fb: Rework locking to fix lock ordering on takeover
>     
>     Adjust the console layer to allow a take over call where the caller already
>     holds the locks. Make the fb layer lock in order.
>     
>     This s partly a band aid, the fb layer is terminally confused about the
>     locking rules it uses for its notifiers it seems.
>     
>     Signed-off-by: Alan Cox <alan@linux.intel.com>

So I went to test this, but first tried to reproduce the orginal lockdep
splat that had irritated me so, and was utterly unsuccessful.  So although
I am now running happily with your patch applied, no ill effects observed,
this gives no confidence because I cannot reproduce the condition anyway.

Sorry to be so unhelpful, original splat without your patch below.

Ah, now I actually scan through it, I see references to blank screen:
I'll try taking off your patch and seeing if it came up at screen
blanking time, then put on your patch back on and try again.
I'll report back in an hour or two.

Hugh

===========================
[ INFO: possible circular locking dependency detected ]
3.7.0-rc3-mm1 #2 Tainted: G        W   
-------------------------------------------------------
kworker/0:1/30 is trying to acquire lock:
ACPI: Invalid Power Resource to register!
 ((fb_notifier_list).rwsem){.+.+.+}, at: [<ffffffff81080ed0>] __blocking_notifier_call_chain+0x6b/0xa2

but task is already holding lock:
 (console_lock){+.+.+.}, at: [<ffffffff812877df>] console_callback+0xc/0xf7

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (console_lock){+.+.+.}:
       [<ffffffff810a517a>] __lock_acquire+0x7fc/0x8bb
       [<ffffffff810a561c>] lock_acquire+0x57/0x6d
       [<ffffffff8106074d>] console_lock+0x67/0x69
       [<ffffffff8128599a>] register_con_driver+0x36/0x128
       [<ffffffff81285e60>] take_over_console+0x21/0x2b7
       [<ffffffff81237014>] fbcon_takeover+0x56/0x98
       [<ffffffff8123a7dd>] fbcon_event_notify+0x3bb/0x6ee
       [<ffffffff81080c32>] notifier_call_chain+0xa7/0xd4
       [<ffffffff81080ee6>] __blocking_notifier_call_chain+0x81/0xa2
       [<ffffffff81080f16>] blocking_notifier_call_chain+0xf/0x11
       [<ffffffff8122f7aa>] fb_notifier_call_chain+0x16/0x18
       [<ffffffff81231523>] register_framebuffer+0x20c/0x270
       [<ffffffff81297357>] drm_fb_helper_single_fb_probe+0x1ce/0x270
       [<ffffffff812975c3>] drm_fb_helper_initial_config+0x1ca/0x1e1
       [<ffffffff812e80bd>] intel_fbdev_init+0x76/0x89
       [<ffffffff812b430a>] i915_driver_load+0xb20/0xcf7
       [<ffffffff812a346e>] drm_get_pci_dev+0x162/0x25b
       [<ffffffff8150f1b8>] i915_pci_probe+0x60/0x69
       [<ffffffff812269cf>] local_pci_probe+0x12/0x16
       [<ffffffff812274a1>] pci_device_probe+0xbe/0xeb
       [<ffffffff812f783d>] driver_probe_device+0x91/0x19e
       [<ffffffff812f79a7>] __driver_attach+0x5d/0x80
       [<ffffffff812f5fac>] bus_for_each_dev+0x52/0x84
       [<ffffffff812f74ed>] driver_attach+0x19/0x1b
       [<ffffffff812f701f>] bus_add_driver+0xe7/0x20c
       [<ffffffff812f7f1b>] driver_register+0x8e/0x114
       [<ffffffff81227590>] __pci_register_driver+0x5a/0x5f
       [<ffffffff812a35e7>] drm_pci_init+0x80/0xe5
       [<ffffffff818c1229>] i915_init+0x66/0x68
       [<ffffffff81000231>] do_one_initcall+0x7a/0x131
       [<ffffffff81508129>] kernel_init+0x106/0x26d
       [<ffffffff81529cac>] ret_from_fork+0x7c/0xb0

-> #0 ((fb_notifier_list).rwsem){.+.+.+}:
       [<ffffffff810a3e14>] validate_chain.isra.21+0x7b0/0xd45
       [<ffffffff810a517a>] __lock_acquire+0x7fc/0x8bb
       [<ffffffff810a561c>] lock_acquire+0x57/0x6d
       [<ffffffff81526df5>] down_read+0x42/0x57
       [<ffffffff81080ed0>] __blocking_notifier_call_chain+0x6b/0xa2
       [<ffffffff81080f16>] blocking_notifier_call_chain+0xf/0x11
       [<ffffffff8122f7aa>] fb_notifier_call_chain+0x16/0x18
       [<ffffffff8122fe47>] fb_blank+0x36/0x85
       [<ffffffff81237ceb>] fbcon_blank+0x129/0x269
       [<ffffffff8128589b>] do_blank_screen+0x18a/0x253
       [<ffffffff8128789f>] console_callback+0xcc/0xf7
       [<ffffffff81076a5e>] process_one_work+0x20e/0x3a2
       [<ffffffff81076e0a>] worker_thread+0x1ee/0x2cb
       [<ffffffff8107b90a>] kthread+0xd0/0xd8
       [<ffffffff81529cac>] ret_from_fork+0x7c/0xb0

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(console_lock);
                               lock((fb_notifier_list).rwsem);
                               lock(console_lock);
  lock((fb_notifier_list).rwsem);

 *** DEADLOCK ***

3 locks held by kworker/0:1/30:
 #0:  (events){.+.+.+}, at: [<ffffffff810769f6>] process_one_work+0x1a6/0x3a2
 #1:  (console_work){+.+.+.}, at: [<ffffffff810769f6>] process_one_work+0x1a6/0x3a2
 #2:  (console_lock){+.+.+.}, at: [<ffffffff812877df>] console_callback+0xc/0xf7

stack backtrace:
Pid: 30, comm: kworker/0:1 Tainted: G        W    3.7.0-rc3-mm1 #2
Call Trace:
 [<ffffffff8151e443>] print_circular_bug+0x28d/0x29e
 [<ffffffff810a3e14>] validate_chain.isra.21+0x7b0/0xd45
 [<ffffffff8107928b>] ? __kernel_text_address+0x22/0x41
 [<ffffffff810a517a>] __lock_acquire+0x7fc/0x8bb
 [<ffffffff810a561c>] lock_acquire+0x57/0x6d
 [<ffffffff81080ed0>] ? __blocking_notifier_call_chain+0x6b/0xa2
 [<ffffffff81526df5>] down_read+0x42/0x57
 [<ffffffff81080ed0>] ? __blocking_notifier_call_chain+0x6b/0xa2
 [<ffffffff81080ed0>] __blocking_notifier_call_chain+0x6b/0xa2
 [<ffffffff81080f16>] blocking_notifier_call_chain+0xf/0x11
 [<ffffffff8122f7aa>] fb_notifier_call_chain+0x16/0x18
 [<ffffffff8122fe47>] fb_blank+0x36/0x85
 [<ffffffff81237ceb>] fbcon_blank+0x129/0x269
 [<ffffffff8152910b>] ? _raw_spin_unlock_irqrestore+0x3a/0x64
 [<ffffffff810a5dbf>] ? trace_hardirqs_on_caller+0x114/0x170
 [<ffffffff810a5e28>] ? trace_hardirqs_on+0xd/0xf
 [<ffffffff81529117>] ? _raw_spin_unlock_irqrestore+0x46/0x64
 [<ffffffff8106bc00>] ? try_to_del_timer_sync+0x48/0x54
 [<ffffffff8106bc77>] ? del_timer_sync+0x6b/0xb4
 [<ffffffff8106bc9a>] ? del_timer_sync+0x8e/0xb4
 [<ffffffff8106bc0c>] ? try_to_del_timer_sync+0x54/0x54
 [<ffffffff8128589b>] do_blank_screen+0x18a/0x253
 [<ffffffff8128789f>] console_callback+0xcc/0xf7
 [<ffffffff810769f6>] ? process_one_work+0x1a6/0x3a2
 [<ffffffff81076a5e>] process_one_work+0x20e/0x3a2
 [<ffffffff810769f6>] ? process_one_work+0x1a6/0x3a2
 [<ffffffff812877d3>] ? poke_blanked_console+0xc9/0xc9
 [<ffffffff81076e0a>] worker_thread+0x1ee/0x2cb
 [<ffffffff81076c1c>] ? process_scheduled_works+0x2a/0x2a
 [<ffffffff8107b90a>] kthread+0xd0/0xd8
 [<ffffffff8152915d>] ? _raw_spin_unlock_irq+0x28/0x50
 [<ffffffff8107b83a>] ? __init_kthread_worker+0x55/0x55
 [<ffffffff81529cac>] ret_from_fork+0x7c/0xb0
 [<ffffffff8107b83a>] ? __init_kthread_worker+0x55/0x55

^ permalink raw reply

* Re: tty, vt: lockdep warnings
From: Hugh Dickins @ 2012-11-07  6:26 UTC (permalink / raw)
  To: Alan Cox
  Cc: Sasha Levin, Daniel Vetter, Sasha Levin, Greg Kroah-Hartman,
	Jiri Slaby, linux-kernel, Dave Jones, linux-fbdev,
	florianSchandinat
In-Reply-To: <alpine.LNX.2.00.1211061950020.1712@eggly.anvils>

On Tue, 6 Nov 2012, Hugh Dickins wrote:
> 
> Ah, now I actually scan through it, I see references to blank screen:
> I'll try taking off your patch and seeing if it came up at screen
> blanking time, then put on your patch back on and try again.
> I'll report back in an hour or two.

Yes, that was it.  When the console screen blanked on 3.7.0-rc3-mm1,
it generated that lockdep splat, visible once I unblanked.  But once I
applied your patch to the kernel, lockdep kept quiet across blank/unblank.

Thanks!
Hugh

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox