[PATCH] drm/i915/sdvo: Reorder i2c initialisation before ddc proxy
Chris Wilson
certain configurations, the i2c-adapter functionality is queried during
initialisation and so may trigger an OOPS during boot. Hence, we need to
reorder the initialisation of the ddc proxy until after we hook up the i2c
adapter for the SDVO device.
Reported-by: Mihai Moldovan <io...@ionic.de>
Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_sdvo.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 4324f33..754086f 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2544,21 +2544,19 @@ bool intel_sdvo_init(struct drm_device *dev, int sdvo_reg)
if (!intel_sdvo)
return false;
+ intel_sdvo->sdvo_reg = sdvo_reg;
+ intel_sdvo->slave_addr = intel_sdvo_get_slave_addr(dev, sdvo_reg) >> 1;
+ intel_sdvo_select_i2c_bus(dev_priv, intel_sdvo, sdvo_reg);
if (!intel_sdvo_init_ddc_proxy(intel_sdvo, dev)) {
kfree(intel_sdvo);
return false;
}
- intel_sdvo->sdvo_reg = sdvo_reg;
-
+ /* encoder type will be decided later */
intel_encoder = &intel_sdvo->base;
intel_encoder->type = INTEL_OUTPUT_SDVO;
- /* encoder type will be decided later */
drm_encoder_init(dev, &intel_encoder->base, &intel_sdvo_enc_funcs, 0);
- intel_sdvo->slave_addr = intel_sdvo_get_slave_addr(dev, sdvo_reg) >> 1;
- intel_sdvo_select_i2c_bus(dev_priv, intel_sdvo, sdvo_reg);
-
/* Read the regs to test if we can talk to the device */
for (i = 0; i < 0x40; i++) {
u8 byte;
--
1.7.5.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Keith Packard
> The ddc proxy depends upon the underlying i2c bus being selected. Under
> certain configurations, the i2c-adapter functionality is queried during
> initialisation and so may trigger an OOPS during boot. Hence, we need to
> reorder the initialisation of the ddc proxy until after we hook up the i2c
> adapter for the SDVO device.
I'd love more explanation here about how this code ever worked -- what
are these 'certain configurations' of which you speak?
(on the surface, this patch seems sane, but I'd love to review with
specific knowledge about what configurations would have worked and which
would have triggered this bug).
Chris Wilson
> On Tue, 17 May 2011 14:03:50 +0100, Chris Wilson <ch...@chris-wilson.co.uk> wrote:
> > The ddc proxy depends upon the underlying i2c bus being selected. Under
> > certain configurations, the i2c-adapter functionality is queried during
> > initialisation and so may trigger an OOPS during boot. Hence, we need to
> > reorder the initialisation of the ddc proxy until after we hook up the i2c
> > adapter for the SDVO device.
>
> I'd love more explanation here about how this code ever worked -- what
> are these 'certain configurations' of which you speak?
The condition under which it fails is when the i2c_add_adapter calls into
i2c_detect which will attempt to probe all valid addresses on the adapter
iff there is a pre-existing i2c_driver with the same class as the freshly
added i2c_adapter.
So it appears to depend upon having compiled in (or loaded such a module
before i915.ko) an i2c-driver that likes to futz over the i2c_adapters
claiming DDC support.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
Keith Packard
> On Tue, 17 May 2011 18:22:52 -0700, Keith Packard <kei...@keithp.com> wrote:
> > On Tue, 17 May 2011 14:03:50 +0100, Chris Wilson <ch...@chris-wilson.co.uk> wrote:
> > > The ddc proxy depends upon the underlying i2c bus being selected. Under
> > > certain configurations, the i2c-adapter functionality is queried during
> > > initialisation and so may trigger an OOPS during boot. Hence, we need to
> > > reorder the initialisation of the ddc proxy until after we hook up the i2c
> > > adapter for the SDVO device.
> >
> > I'd love more explanation here about how this code ever worked -- what
> > are these 'certain configurations' of which you speak?
>
> The condition under which it fails is when the i2c_add_adapter calls into
> i2c_detect which will attempt to probe all valid addresses on the adapter
> iff there is a pre-existing i2c_driver with the same class as the freshly
> added i2c_adapter.
Lovely.
> So it appears to depend upon having compiled in (or loaded such a module
> before i915.ko) an i2c-driver that likes to futz over the i2c_adapters
> claiming DDC support.
Thanks for your additional explanation, as I said, the patch looks sane
on the face of it (always nice to initialize values before use).
I've marked this as reviewed, added your additional explanation and
merged it to drm-intel-next. Seems like it might like a cc to stable?