* [PATCH 1/5] drm/i915: redisable VGA when we disable the power well
2013-09-26 23:05 [PATCH 0/5] module_reload fixes Paulo Zanoni
@ 2013-09-26 23:05 ` Paulo Zanoni
2013-10-01 13:47 ` Ville Syrjälä
2013-09-26 23:05 ` [PATCH 2/5] drm/i915: destroy connector sysfs files earlier Paulo Zanoni
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2013-09-26 23:05 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, dri-devel
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
VGA whack-a-mole!
We need VGA to be disabled whenever our driver is working. So even
without reproducible bugs this patch makes sense, but we do have a bug
solved by this patch.
If you boot a Haswell machine with eDP+HDMI, then kill your display
manager and run:
echo 0 > /sys/class/vtconsole/vtcon1/bind
you'll get thousands of "unclaimed register" messages. Notice that
since we have eDP+HDMI, the power well is *enabled* when we run the
command, but if you look at dmesg you'll see that at some point during
the boot we disabled it and then reenabled it. This patch solves this
problem.
I didn't do a deep analysis of the problem, but I guess vgacon gets
seriously confused when it sees that the VGA plane is enabled.
Besides the command above, this problem can also be reproduced by the
"module_reload" test from intel-gpu-tools.
Fixes regression introduced by:
commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41
Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
drm/i915: switch disable_power_well default value to 1
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67813
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 2 +-
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 665fa8f..065ffed 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10232,7 +10232,7 @@ static void intel_init_quirks(struct drm_device *dev)
}
/* Disable the VGA plane that we never use */
-static void i915_disable_vga(struct drm_device *dev)
+void i915_disable_vga(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
u8 sr1;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a17a86a..e63646a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -678,6 +678,7 @@ void
ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
int dotclock);
bool intel_crtc_active(struct drm_crtc *crtc);
+void i915_disable_vga(struct drm_device *dev);
void i915_disable_vga_mem(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2ac1c2f..7a8af95 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5329,6 +5329,12 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable)
if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) &
HSW_PWR_WELL_STATE_ENABLED), 20))
DRM_ERROR("Timeout enabling power well\n");
+
+ if (I915_READ(i915_vgacntrl_reg(dev)) !=
+ VGA_DISP_DISABLE) {
+ i915_disable_vga(dev);
+ i915_disable_vga_mem(dev);
+ }
}
} else {
if (enable_requested) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/5] drm/i915: redisable VGA when we disable the power well
2013-09-26 23:05 ` [PATCH 1/5] drm/i915: redisable VGA when we disable the power well Paulo Zanoni
@ 2013-10-01 13:47 ` Ville Syrjälä
2013-10-01 18:18 ` Ville Syrjälä
0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2013-10-01 13:47 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni, dri-devel
On Thu, Sep 26, 2013 at 08:05:58PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> VGA whack-a-mole!
>
> We need VGA to be disabled whenever our driver is working. So even
> without reproducible bugs this patch makes sense, but we do have a bug
> solved by this patch.
>
> If you boot a Haswell machine with eDP+HDMI, then kill your display
> manager and run:
> echo 0 > /sys/class/vtconsole/vtcon1/bind
> you'll get thousands of "unclaimed register" messages. Notice that
> since we have eDP+HDMI, the power well is *enabled* when we run the
> command, but if you look at dmesg you'll see that at some point during
> the boot we disabled it and then reenabled it. This patch solves this
> problem.
>
> I didn't do a deep analysis of the problem, but I guess vgacon gets
> seriously confused when it sees that the VGA plane is enabled.
>
> Besides the command above, this problem can also be reproduced by the
> "module_reload" test from intel-gpu-tools.
>
> Fixes regression introduced by:
> commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41
> Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> drm/i915: switch disable_power_well default value to 1
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67813
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
> 3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 665fa8f..065ffed 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10232,7 +10232,7 @@ static void intel_init_quirks(struct drm_device *dev)
> }
>
> /* Disable the VGA plane that we never use */
> -static void i915_disable_vga(struct drm_device *dev)
> +void i915_disable_vga(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> u8 sr1;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a17a86a..e63646a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -678,6 +678,7 @@ void
> ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
> int dotclock);
> bool intel_crtc_active(struct drm_crtc *crtc);
> +void i915_disable_vga(struct drm_device *dev);
> void i915_disable_vga_mem(struct drm_device *dev);
>
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2ac1c2f..7a8af95 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5329,6 +5329,12 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable)
> if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) &
> HSW_PWR_WELL_STATE_ENABLED), 20))
> DRM_ERROR("Timeout enabling power well\n");
> +
> + if (I915_READ(i915_vgacntrl_reg(dev)) !=
> + VGA_DISP_DISABLE) {
> + i915_disable_vga(dev);
> + i915_disable_vga_mem(dev);
> + }
Hmm. That makes no sense.
So somehow the thrice cursed BIOS re-enabled the VGA plane behind our
back? That really shouldn't cause any problems other than maybe
corrupting the display or something. In any case it has nothing to do
with vga resource decode.
The real problem seems to be that vgacon is poking at vga resources we
apparently decode but can't handle. If your patch really fixes it then
it would seem to BIOS also enabled VGA memory decode. But that still
doesn't explain the unclaimed register errors, as that would require
the power well to be off. But since you say HDMI is enabled, the power
well must be on as well. So all I can say is wtf?
> }
> } else {
> if (enable_requested) {
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/5] drm/i915: redisable VGA when we disable the power well
2013-10-01 13:47 ` Ville Syrjälä
@ 2013-10-01 18:18 ` Ville Syrjälä
0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2013-10-01 18:18 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni, dri-devel
On Tue, Oct 01, 2013 at 04:47:04PM +0300, Ville Syrjälä wrote:
> On Thu, Sep 26, 2013 at 08:05:58PM -0300, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > VGA whack-a-mole!
> >
> > We need VGA to be disabled whenever our driver is working. So even
> > without reproducible bugs this patch makes sense, but we do have a bug
> > solved by this patch.
> >
> > If you boot a Haswell machine with eDP+HDMI, then kill your display
> > manager and run:
> > echo 0 > /sys/class/vtconsole/vtcon1/bind
> > you'll get thousands of "unclaimed register" messages. Notice that
> > since we have eDP+HDMI, the power well is *enabled* when we run the
> > command, but if you look at dmesg you'll see that at some point during
> > the boot we disabled it and then reenabled it. This patch solves this
> > problem.
> >
> > I didn't do a deep analysis of the problem, but I guess vgacon gets
> > seriously confused when it sees that the VGA plane is enabled.
> >
> > Besides the command above, this problem can also be reproduced by the
> > "module_reload" test from intel-gpu-tools.
> >
> > Fixes regression introduced by:
> > commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41
> > Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > drm/i915: switch disable_power_well default value to 1
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67813
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 2 +-
> > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
> > 3 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 665fa8f..065ffed 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10232,7 +10232,7 @@ static void intel_init_quirks(struct drm_device *dev)
> > }
> >
> > /* Disable the VGA plane that we never use */
> > -static void i915_disable_vga(struct drm_device *dev)
> > +void i915_disable_vga(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > u8 sr1;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index a17a86a..e63646a 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -678,6 +678,7 @@ void
> > ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
> > int dotclock);
> > bool intel_crtc_active(struct drm_crtc *crtc);
> > +void i915_disable_vga(struct drm_device *dev);
> > void i915_disable_vga_mem(struct drm_device *dev);
> >
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 2ac1c2f..7a8af95 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5329,6 +5329,12 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable)
> > if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) &
> > HSW_PWR_WELL_STATE_ENABLED), 20))
> > DRM_ERROR("Timeout enabling power well\n");
> > +
> > + if (I915_READ(i915_vgacntrl_reg(dev)) !=
> > + VGA_DISP_DISABLE) {
> > + i915_disable_vga(dev);
> > + i915_disable_vga_mem(dev);
> > + }
>
> Hmm. That makes no sense.
>
> So somehow the thrice cursed BIOS re-enabled the VGA plane behind our
> back? That really shouldn't cause any problems other than maybe
> corrupting the display or something. In any case it has nothing to do
> with vga resource decode.
>
> The real problem seems to be that vgacon is poking at vga resources we
> apparently decode but can't handle. If your patch really fixes it then
> it would seem to BIOS also enabled VGA memory decode. But that still
> doesn't explain the unclaimed register errors, as that would require
> the power well to be off. But since you say HDMI is enabled, the power
> well must be on as well. So all I can say is wtf?
Actually I tried it on my HSW, and the vga_cntrl reg always comes back
as 0x80002900 after the power well has been off, which is the reset
value. So your code w/ the '!=' comparoson would assume that the VGA plane
is enabled even though it might not be. We should fix all the VGA plane
enabled checks to use '&' instead of '!='.
But if your patch actually fixes things, then I guess the BIOS really
must have re-enabled vga memory decode and that's where the errors
are coming from. Otherwise if it's just about vga registers, then
redisabling vga memory decode and vga plane should have no effect on
the outcome.
One idea would be to lift the lid_notify hack from intel_lvds.c and plug
it into eDP and see if that makes a difference...
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/5] drm/i915: destroy connector sysfs files earlier
2013-09-26 23:05 [PATCH 0/5] module_reload fixes Paulo Zanoni
2013-09-26 23:05 ` [PATCH 1/5] drm/i915: redisable VGA when we disable the power well Paulo Zanoni
@ 2013-09-26 23:05 ` Paulo Zanoni
2013-09-30 21:10 ` Daniel Vetter
2013-09-26 23:06 ` [PATCH 3/5] tty/vt: add con_bind and con_unbind functions Paulo Zanoni
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2013-09-26 23:05 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, dri-devel
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
For some reason, every single time I try to run module_reload
something tries to read the connector sysfs files. This happens
after we destroy the encoders and before we destroy the connectors, so
when the sysfs read triggers the connector detect() function,
intel_conector->encoder points to memory that was already freed.
The bad backtrace is just:
[<ffffffff8163ca9a>] dump_stack+0x54/0x74
[<ffffffffa00c2c8e>] intel_dp_detect+0x1e/0x4b0 [i915]
[<ffffffffa001913d>] status_show+0x3d/0x80 [drm]
[<ffffffff813d5340>] dev_attr_show+0x20/0x60
[<ffffffff81221f50>] ? sysfs_read_file+0x80/0x1b0
[<ffffffff81221f79>] sysfs_read_file+0xa9/0x1b0
[<ffffffff811aaf1e>] vfs_read+0x9e/0x170
[<ffffffff811aba4c>] SyS_read+0x4c/0xa0
[<ffffffff8164e392>] system_call_fastpath+0x16/0x1b
But if you add tons of memory checking debug options to your Kernel
you'll also see:
- general protection fault: 0000
- BUG kmalloc-4096 (Tainted: G D W ): Poison overwritten
- INFO: Allocated in intel_ddi_init+0x65/0x270 [i915]
- INFO: Freed in intel_dp_encoder_destroy+0x69/0xb0 [i915]
Among a bunch of other error messages.
So this commit just destroys the sysfs files before both the encoder
and connectors are freed.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_crt.c | 1 -
drivers/gpu/drm/i915/intel_display.c | 5 +++++
drivers/gpu/drm/i915/intel_dp.c | 1 -
drivers/gpu/drm/i915/intel_dsi.c | 1 -
drivers/gpu/drm/i915/intel_dvo.c | 1 -
drivers/gpu/drm/i915/intel_hdmi.c | 1 -
drivers/gpu/drm/i915/intel_lvds.c | 1 -
drivers/gpu/drm/i915/intel_sdvo.c | 7 +++++--
drivers/gpu/drm/i915/intel_tv.c | 1 -
9 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 0263629..942b9ac 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -677,7 +677,6 @@ intel_crt_detect(struct drm_connector *connector, bool force)
static void intel_crt_destroy(struct drm_connector *connector)
{
- drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
kfree(connector);
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 065ffed..793061f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10726,6 +10726,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *crtc;
+ struct drm_connector *connector;
/*
* Interrupts and polling as the first thing to avoid creating havoc.
@@ -10768,6 +10769,10 @@ void intel_modeset_cleanup(struct drm_device *dev)
/* destroy backlight, if any, before the connectors */
intel_panel_destroy_backlight(dev);
+ /* destroy the sysfs files before encoders/connectors */
+ list_for_each_entry(connector, &dev->mode_config.connector_list, head)
+ drm_sysfs_connector_remove(connector);
+
drm_mode_config_cleanup(dev);
intel_cleanup_overlay(dev);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 95a3159..7bdbb36 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3139,7 +3139,6 @@ intel_dp_connector_destroy(struct drm_connector *connector)
if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
intel_panel_fini(&intel_connector->panel);
- drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
kfree(connector);
}
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 674fd49..9a2fdd2 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -504,7 +504,6 @@ static void intel_dsi_destroy(struct drm_connector *connector)
DRM_DEBUG_KMS("\n");
intel_panel_fini(&intel_connector->panel);
- drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
kfree(connector);
}
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 91287d1..1b64145 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -367,7 +367,6 @@ static int intel_dvo_get_modes(struct drm_connector *connector)
static void intel_dvo_destroy(struct drm_connector *connector)
{
- drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
kfree(connector);
}
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 6004f9c..4f4d346 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1181,7 +1181,6 @@ static void intel_hdmi_post_disable(struct intel_encoder *encoder)
static void intel_hdmi_destroy(struct drm_connector *connector)
{
- drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
kfree(connector);
}
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index fb3f8ef..ae0c843 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -474,7 +474,6 @@ static void intel_lvds_destroy(struct drm_connector *connector)
intel_panel_fini(&lvds_connector->base.panel);
- drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
kfree(connector);
}
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 5e59d64..a583e8f 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2009,7 +2009,6 @@ static void intel_sdvo_destroy(struct drm_connector *connector)
intel_sdvo_connector->tv_format);
intel_sdvo_destroy_enhance_property(connector);
- drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
kfree(intel_sdvo_connector);
}
@@ -2482,6 +2481,7 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
return true;
err:
+ drm_sysfs_connector_remove(connector);
intel_sdvo_destroy(connector);
return false;
}
@@ -2553,6 +2553,7 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
return true;
err:
+ drm_sysfs_connector_remove(connector);
intel_sdvo_destroy(connector);
return false;
}
@@ -2624,8 +2625,10 @@ static void intel_sdvo_output_cleanup(struct intel_sdvo *intel_sdvo)
list_for_each_entry_safe(connector, tmp,
&dev->mode_config.connector_list, head) {
- if (intel_attached_encoder(connector) == &intel_sdvo->base)
+ if (intel_attached_encoder(connector) == &intel_sdvo->base) {
+ drm_sysfs_connector_remove(connector);
intel_sdvo_destroy(connector);
+ }
}
}
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 75925a1..92895f9 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1433,7 +1433,6 @@ intel_tv_get_modes(struct drm_connector *connector)
static void
intel_tv_destroy(struct drm_connector *connector)
{
- drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
kfree(connector);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/5] drm/i915: destroy connector sysfs files earlier
2013-09-26 23:05 ` [PATCH 2/5] drm/i915: destroy connector sysfs files earlier Paulo Zanoni
@ 2013-09-30 21:10 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2013-09-30 21:10 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni, dri-devel
On Thu, Sep 26, 2013 at 08:05:59PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> For some reason, every single time I try to run module_reload
> something tries to read the connector sysfs files. This happens
> after we destroy the encoders and before we destroy the connectors, so
> when the sysfs read triggers the connector detect() function,
> intel_conector->encoder points to memory that was already freed.
>
> The bad backtrace is just:
> [<ffffffff8163ca9a>] dump_stack+0x54/0x74
> [<ffffffffa00c2c8e>] intel_dp_detect+0x1e/0x4b0 [i915]
> [<ffffffffa001913d>] status_show+0x3d/0x80 [drm]
> [<ffffffff813d5340>] dev_attr_show+0x20/0x60
> [<ffffffff81221f50>] ? sysfs_read_file+0x80/0x1b0
> [<ffffffff81221f79>] sysfs_read_file+0xa9/0x1b0
> [<ffffffff811aaf1e>] vfs_read+0x9e/0x170
> [<ffffffff811aba4c>] SyS_read+0x4c/0xa0
> [<ffffffff8164e392>] system_call_fastpath+0x16/0x1b
>
> But if you add tons of memory checking debug options to your Kernel
> you'll also see:
> - general protection fault: 0000
> - BUG kmalloc-4096 (Tainted: G D W ): Poison overwritten
> - INFO: Allocated in intel_ddi_init+0x65/0x270 [i915]
> - INFO: Freed in intel_dp_encoder_destroy+0x69/0xb0 [i915]
> Among a bunch of other error messages.
>
> So this commit just destroys the sysfs files before both the encoder
> and connectors are freed.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Queued for -next, thanks for the patch. I'm unsure what to do with the
first patch, maybe Ville can take a look. For the next three patches to
wire up bind/unbind hooks I think the right approach would be to just
disallow the vgaconsole once drm/i915 is loaded:
- We don't want to even tighter integrate with the locking madness that is
fbcon.
- We want to kick out the vgacon anyway, even when e.g. fbcon isn't
loaded.
David Herrmann might have an idea how to solve this.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_crt.c | 1 -
> drivers/gpu/drm/i915/intel_display.c | 5 +++++
> drivers/gpu/drm/i915/intel_dp.c | 1 -
> drivers/gpu/drm/i915/intel_dsi.c | 1 -
> drivers/gpu/drm/i915/intel_dvo.c | 1 -
> drivers/gpu/drm/i915/intel_hdmi.c | 1 -
> drivers/gpu/drm/i915/intel_lvds.c | 1 -
> drivers/gpu/drm/i915/intel_sdvo.c | 7 +++++--
> drivers/gpu/drm/i915/intel_tv.c | 1 -
> 9 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 0263629..942b9ac 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -677,7 +677,6 @@ intel_crt_detect(struct drm_connector *connector, bool force)
>
> static void intel_crt_destroy(struct drm_connector *connector)
> {
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(connector);
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 065ffed..793061f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10726,6 +10726,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_crtc *crtc;
> + struct drm_connector *connector;
>
> /*
> * Interrupts and polling as the first thing to avoid creating havoc.
> @@ -10768,6 +10769,10 @@ void intel_modeset_cleanup(struct drm_device *dev)
> /* destroy backlight, if any, before the connectors */
> intel_panel_destroy_backlight(dev);
>
> + /* destroy the sysfs files before encoders/connectors */
> + list_for_each_entry(connector, &dev->mode_config.connector_list, head)
> + drm_sysfs_connector_remove(connector);
> +
> drm_mode_config_cleanup(dev);
>
> intel_cleanup_overlay(dev);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 95a3159..7bdbb36 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3139,7 +3139,6 @@ intel_dp_connector_destroy(struct drm_connector *connector)
> if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> intel_panel_fini(&intel_connector->panel);
>
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(connector);
> }
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 674fd49..9a2fdd2 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -504,7 +504,6 @@ static void intel_dsi_destroy(struct drm_connector *connector)
>
> DRM_DEBUG_KMS("\n");
> intel_panel_fini(&intel_connector->panel);
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(connector);
> }
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 91287d1..1b64145 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -367,7 +367,6 @@ static int intel_dvo_get_modes(struct drm_connector *connector)
>
> static void intel_dvo_destroy(struct drm_connector *connector)
> {
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(connector);
> }
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 6004f9c..4f4d346 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1181,7 +1181,6 @@ static void intel_hdmi_post_disable(struct intel_encoder *encoder)
>
> static void intel_hdmi_destroy(struct drm_connector *connector)
> {
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(connector);
> }
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index fb3f8ef..ae0c843 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -474,7 +474,6 @@ static void intel_lvds_destroy(struct drm_connector *connector)
>
> intel_panel_fini(&lvds_connector->base.panel);
>
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(connector);
> }
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 5e59d64..a583e8f 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2009,7 +2009,6 @@ static void intel_sdvo_destroy(struct drm_connector *connector)
> intel_sdvo_connector->tv_format);
>
> intel_sdvo_destroy_enhance_property(connector);
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(intel_sdvo_connector);
> }
> @@ -2482,6 +2481,7 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
> return true;
>
> err:
> + drm_sysfs_connector_remove(connector);
> intel_sdvo_destroy(connector);
> return false;
> }
> @@ -2553,6 +2553,7 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
> return true;
>
> err:
> + drm_sysfs_connector_remove(connector);
> intel_sdvo_destroy(connector);
> return false;
> }
> @@ -2624,8 +2625,10 @@ static void intel_sdvo_output_cleanup(struct intel_sdvo *intel_sdvo)
>
> list_for_each_entry_safe(connector, tmp,
> &dev->mode_config.connector_list, head) {
> - if (intel_attached_encoder(connector) == &intel_sdvo->base)
> + if (intel_attached_encoder(connector) == &intel_sdvo->base) {
> + drm_sysfs_connector_remove(connector);
> intel_sdvo_destroy(connector);
> + }
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 75925a1..92895f9 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1433,7 +1433,6 @@ intel_tv_get_modes(struct drm_connector *connector)
> static void
> intel_tv_destroy(struct drm_connector *connector)
> {
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(connector);
> }
> --
> 1.8.3.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/5] tty/vt: add con_bind and con_unbind functions
2013-09-26 23:05 [PATCH 0/5] module_reload fixes Paulo Zanoni
2013-09-26 23:05 ` [PATCH 1/5] drm/i915: redisable VGA when we disable the power well Paulo Zanoni
2013-09-26 23:05 ` [PATCH 2/5] drm/i915: destroy connector sysfs files earlier Paulo Zanoni
@ 2013-09-26 23:06 ` Paulo Zanoni
2013-10-01 13:50 ` Ville Syrjälä
2013-09-26 23:06 ` [PATCH 4/5] console/fbcon: implement con_bind and con_unbind Paulo Zanoni
2013-09-26 23:06 ` [PATCH 5/5] drm/i915: put/get the power well at the FB bind/unbind functions Paulo Zanoni
4 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2013-09-26 23:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, dri-devel
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
The consoles who need to do something when unbinding or binding can
optionally implement these functions.
The current problem I'm trying to solve is that when i915+fbcon is
loaded on Haswell, if we disable the power well (to save power) the
VGA interface gets completely disabled, so when we unbind fbcon we
need to restore the VGA interface to allow vgacon to work.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/tty/vt/vt.c | 6 ++++++
include/linux/console.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 9a8e8c5..beb5986 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3014,6 +3014,9 @@ static int do_bind_con_driver(const struct consw *csw, int first, int last,
if (retval)
goto err;
+ if (csw->con_bind)
+ csw->con_bind();
+
if (!(con_driver->flag & CON_DRIVER_FLAG_INIT)) {
csw->con_startup();
con_driver->flag |= CON_DRIVER_FLAG_INIT;
@@ -3152,6 +3155,9 @@ int do_unbind_con_driver(const struct consw *csw, int first, int last, int deflt
if (!con_is_bound(csw))
goto err;
+ if (csw->con_unbind)
+ csw->con_unbind();
+
first = max(first, con_driver->first);
last = min(last, con_driver->last);
diff --git a/include/linux/console.h b/include/linux/console.h
index 7571a16..5cd2c35 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -65,6 +65,8 @@ struct consw {
* Restore the console to its pre-debug state as closely as possible.
*/
int (*con_debug_leave)(struct vc_data *);
+ void (*con_bind)(void);
+ void (*con_unbind)(void);
};
extern const struct consw *conswitchp;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] tty/vt: add con_bind and con_unbind functions
2013-09-26 23:06 ` [PATCH 3/5] tty/vt: add con_bind and con_unbind functions Paulo Zanoni
@ 2013-10-01 13:50 ` Ville Syrjälä
2013-10-08 21:12 ` Paulo Zanoni
0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2013-10-01 13:50 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni, dri-devel
On Thu, Sep 26, 2013 at 08:06:00PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> The consoles who need to do something when unbinding or binding can
> optionally implement these functions.
>
> The current problem I'm trying to solve is that when i915+fbcon is
> loaded on Haswell, if we disable the power well (to save power) the
> VGA interface gets completely disabled, so when we unbind fbcon we
> need to restore the VGA interface to allow vgacon to work.
We don't need to make it work. No one else does, and in the general case
it's even impossible since on some hardware that would definitely
corrupt the state that the real driver is attempting to use. The only
case where it might be nice to restore vgacon is on i915 unload, but no
one else does that either AFAIK, so I would not waste any cycles on
attempting that.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/tty/vt/vt.c | 6 ++++++
> include/linux/console.h | 2 ++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 9a8e8c5..beb5986 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -3014,6 +3014,9 @@ static int do_bind_con_driver(const struct consw *csw, int first, int last,
> if (retval)
> goto err;
>
> + if (csw->con_bind)
> + csw->con_bind();
> +
> if (!(con_driver->flag & CON_DRIVER_FLAG_INIT)) {
> csw->con_startup();
> con_driver->flag |= CON_DRIVER_FLAG_INIT;
> @@ -3152,6 +3155,9 @@ int do_unbind_con_driver(const struct consw *csw, int first, int last, int deflt
> if (!con_is_bound(csw))
> goto err;
>
> + if (csw->con_unbind)
> + csw->con_unbind();
> +
> first = max(first, con_driver->first);
> last = min(last, con_driver->last);
>
> diff --git a/include/linux/console.h b/include/linux/console.h
> index 7571a16..5cd2c35 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -65,6 +65,8 @@ struct consw {
> * Restore the console to its pre-debug state as closely as possible.
> */
> int (*con_debug_leave)(struct vc_data *);
> + void (*con_bind)(void);
> + void (*con_unbind)(void);
> };
>
> extern const struct consw *conswitchp;
> --
> 1.8.3.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] tty/vt: add con_bind and con_unbind functions
2013-10-01 13:50 ` Ville Syrjälä
@ 2013-10-08 21:12 ` Paulo Zanoni
2013-10-09 8:10 ` Ville Syrjälä
0 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2013-10-08 21:12 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Intel Graphics Development, Paulo Zanoni, DRI Development
2013/10/1 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> On Thu, Sep 26, 2013 at 08:06:00PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> The consoles who need to do something when unbinding or binding can
>> optionally implement these functions.
>>
>> The current problem I'm trying to solve is that when i915+fbcon is
>> loaded on Haswell, if we disable the power well (to save power) the
>> VGA interface gets completely disabled, so when we unbind fbcon we
>> need to restore the VGA interface to allow vgacon to work.
>
> We don't need to make it work. No one else does, and in the general case
> it's even impossible since on some hardware that would definitely
> corrupt the state that the real driver is attempting to use. The only
> case where it might be nice to restore vgacon is on i915 unload, but no
> one else does that either AFAIK, so I would not waste any cycles on
> attempting that.
I don't understand your point. Without patches 3-4-5, module_reload
doesn't work at all if the power well is disabled: we need these
patches to fix it. The plan is not to restore everything to make
vgacon actually work, the plan is just to prevent it from breaking
module_reload.
>
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>> drivers/tty/vt/vt.c | 6 ++++++
>> include/linux/console.h | 2 ++
>> 2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
>> index 9a8e8c5..beb5986 100644
>> --- a/drivers/tty/vt/vt.c
>> +++ b/drivers/tty/vt/vt.c
>> @@ -3014,6 +3014,9 @@ static int do_bind_con_driver(const struct consw *csw, int first, int last,
>> if (retval)
>> goto err;
>>
>> + if (csw->con_bind)
>> + csw->con_bind();
>> +
>> if (!(con_driver->flag & CON_DRIVER_FLAG_INIT)) {
>> csw->con_startup();
>> con_driver->flag |= CON_DRIVER_FLAG_INIT;
>> @@ -3152,6 +3155,9 @@ int do_unbind_con_driver(const struct consw *csw, int first, int last, int deflt
>> if (!con_is_bound(csw))
>> goto err;
>>
>> + if (csw->con_unbind)
>> + csw->con_unbind();
>> +
>> first = max(first, con_driver->first);
>> last = min(last, con_driver->last);
>>
>> diff --git a/include/linux/console.h b/include/linux/console.h
>> index 7571a16..5cd2c35 100644
>> --- a/include/linux/console.h
>> +++ b/include/linux/console.h
>> @@ -65,6 +65,8 @@ struct consw {
>> * Restore the console to its pre-debug state as closely as possible.
>> */
>> int (*con_debug_leave)(struct vc_data *);
>> + void (*con_bind)(void);
>> + void (*con_unbind)(void);
>> };
>>
>> extern const struct consw *conswitchp;
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel OTC
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] tty/vt: add con_bind and con_unbind functions
2013-10-08 21:12 ` Paulo Zanoni
@ 2013-10-09 8:10 ` Ville Syrjälä
0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2013-10-09 8:10 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development, Paulo Zanoni, DRI Development
On Tue, Oct 08, 2013 at 06:12:15PM -0300, Paulo Zanoni wrote:
> 2013/10/1 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> > On Thu, Sep 26, 2013 at 08:06:00PM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> The consoles who need to do something when unbinding or binding can
> >> optionally implement these functions.
> >>
> >> The current problem I'm trying to solve is that when i915+fbcon is
> >> loaded on Haswell, if we disable the power well (to save power) the
> >> VGA interface gets completely disabled, so when we unbind fbcon we
> >> need to restore the VGA interface to allow vgacon to work.
> >
> > We don't need to make it work. No one else does, and in the general case
> > it's even impossible since on some hardware that would definitely
> > corrupt the state that the real driver is attempting to use. The only
> > case where it might be nice to restore vgacon is on i915 unload, but no
> > one else does that either AFAIK, so I would not waste any cycles on
> > attempting that.
>
> I don't understand your point. Without patches 3-4-5, module_reload
> doesn't work at all if the power well is disabled: we need these
> patches to fix it. The plan is not to restore everything to make
> vgacon actually work, the plan is just to prevent it from breaking
> module_reload.
How does the power well vs. vgacon break module_reload?
BTW module_reload seems to be busted on ILK and IVB too currently.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/5] console/fbcon: implement con_bind and con_unbind
2013-09-26 23:05 [PATCH 0/5] module_reload fixes Paulo Zanoni
` (2 preceding siblings ...)
2013-09-26 23:06 ` [PATCH 3/5] tty/vt: add con_bind and con_unbind functions Paulo Zanoni
@ 2013-09-26 23:06 ` Paulo Zanoni
2013-09-26 23:06 ` [PATCH 5/5] drm/i915: put/get the power well at the FB bind/unbind functions Paulo Zanoni
4 siblings, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2013-09-26 23:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, dri-devel
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
And create fb_bind and fb_unbind fb_ops that the drivers can
implement.
The current problem I'm trying to solve is that when i915+fbcon is
loaded on Haswell, if we disable the power well (to save power) the
VGA interface gets completely disabled, so when we unbind fbcon we
need to restore the VGA interface to allow vgacon to work.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/video/console/fbcon.c | 26 ++++++++++++++++++++++++++
include/linux/fb.h | 4 ++++
2 files changed, 30 insertions(+)
diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index cd8a802..68d316a 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -3298,6 +3298,30 @@ done:
return ret;
}
+static void fbcon_con_bind(void)
+{
+ struct fb_info *info = NULL;
+
+ info = registered_fb[info_idx];
+ if (!info)
+ return;
+
+ if (info->fbops->fb_bind)
+ info->fbops->fb_bind(info);
+}
+
+static void fbcon_con_unbind(void)
+{
+ struct fb_info *info = NULL;
+
+ info = registered_fb[info_idx];
+ if (!info)
+ return;
+
+ if (info->fbops->fb_unbind)
+ info->fbops->fb_unbind(info);
+}
+
/*
* The console `switch' structure for the frame buffer based console
*/
@@ -3328,6 +3352,8 @@ static const struct consw fb_con = {
.con_resize = fbcon_resize,
.con_debug_enter = fbcon_debug_enter,
.con_debug_leave = fbcon_debug_leave,
+ .con_bind = fbcon_con_bind,
+ .con_unbind = fbcon_con_unbind,
};
static struct notifier_block fbcon_event_notifier = {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index ffac70a..8074bd5 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -304,6 +304,10 @@ struct fb_ops {
/* called at KDB enter and leave time to prepare the console */
int (*fb_debug_enter)(struct fb_info *info);
int (*fb_debug_leave)(struct fb_info *info);
+
+ /* called when binding/unbinding */
+ void (*fb_bind)(struct fb_info *info);
+ void (*fb_unbind)(struct fb_info *info);
};
#ifdef CONFIG_FB_TILEBLITTING
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/5] drm/i915: put/get the power well at the FB bind/unbind functions
2013-09-26 23:05 [PATCH 0/5] module_reload fixes Paulo Zanoni
` (3 preceding siblings ...)
2013-09-26 23:06 ` [PATCH 4/5] console/fbcon: implement con_bind and con_unbind Paulo Zanoni
@ 2013-09-26 23:06 ` Paulo Zanoni
4 siblings, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2013-09-26 23:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, dri-devel
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
If we run the following command on Haswell when the power well is
disabled:
echo 0 > /sys/class/vtconsole/vtcon1/bind
then we'll get thousands of consecutive interrupts because something
is trying to touch registers that are on the disabled power well. So
before we unbind the console, we need to enable the power well to make
the VGA interface work correctly. With this, whoever is using it
doesn't trigger interrupts on i915.ko.
Fixes regression introduced by:
commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41
Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
drm/i915: switch disable_power_well default value to 1
Besides the command above, the "module_reload" test from
intel-gpu-tools can also be used to reproduce the bug fixed by this
patch.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67813
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_fb.c | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e63646a..a643c48 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -113,6 +113,7 @@ struct intel_fbdev {
struct intel_framebuffer ifb;
struct list_head fbdev_list;
struct drm_display_mode *our_mode;
+ bool first_bind_done;
};
struct intel_encoder {
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index d883b77..8c6ed1c 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -43,6 +43,27 @@
#include <drm/i915_drm.h>
#include "i915_drv.h"
+static void intel_fb_bind(struct fb_info *info)
+{
+ struct drm_fb_helper *drm_helper = info->par;
+ struct drm_device *dev = drm_helper->dev;
+ struct intel_fbdev *ifbdev =
+ container_of(drm_helper, struct intel_fbdev, helper);
+
+ if (ifbdev->first_bind_done)
+ intel_display_power_put(dev, POWER_DOMAIN_VGA);
+ else
+ ifbdev->first_bind_done = true;
+}
+
+static void intel_fb_unbind(struct fb_info *info)
+{
+ struct drm_fb_helper *fb_helper = info->par;
+ struct drm_device *dev = fb_helper->dev;
+
+ intel_display_power_get(dev, POWER_DOMAIN_VGA);
+}
+
static struct fb_ops intelfb_ops = {
.owner = THIS_MODULE,
.fb_check_var = drm_fb_helper_check_var,
@@ -55,6 +76,8 @@ static struct fb_ops intelfb_ops = {
.fb_setcmap = drm_fb_helper_setcmap,
.fb_debug_enter = drm_fb_helper_debug_enter,
.fb_debug_leave = drm_fb_helper_debug_leave,
+ .fb_bind = intel_fb_bind,
+ .fb_unbind = intel_fb_unbind,
};
static int intelfb_create(struct drm_fb_helper *helper,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread