On Monday 19 November 2007 00:16:06 Benjamin Herrenschmidt wrote: > (Also, Michel, can you check if it fixes your other problem with this > code ? ie. your "hot crash")
> -------- Forwarded Message -------- > From: Jean Delvare <[EMAIL PROTECTED]> > To: [EMAIL PROTECTED] > Cc: Benjamin Herrenschmidt <[EMAIL PROTECTED]>, Antonino Daplas > <[EMAIL PROTECTED]> > Subject: [PATCH] fb_ddc: Fix DDC lines quirk > Date: Sun, 18 Nov 2007 14:21:41 +0100 > > The code in fb_ddc_read() is said to be based on the implementation > of the radeon driver: > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=fc5891c8a3ba284f13994d7bc1f1bfa8283982de > > However, comparing the old radeon driver code with the new fb_ddc code > reveals some differences. Most notably, the I2C bus lines are held at > the end of the function, while the original code was releasing them > (as the comment above correctly says.) > > There are a few other differences, which appear to be responsible for > read failures on my system. While tracing low-level I2C code in > i2c-algo-bit, I noticed that the initial attempt to read the EDID > always failed. It takes one retry for the read to succeed. As we are > about to remove this automatic retry property from i2c-algo-bit, > reading the EDID would really fail. > > As a summary, the I2C lines quirk which is supposedly needed to read > EDID on some older monitors is currently breaking the (first) read on > all other monitors (and might not even work with older ones - did > anyone try since October 2006?) > > After applying the patch below, which makes the code in fb_ddc_read() > really similar to what the radeon driver used to have, the first EDID > read succeeds again. > > On top of that, as it appears that this code has been broken for one > year now and nobody seems to have complained, I'm curious if it makes > sense to keep this quirk in place. It makes the code more complex and > slower just for the sake of monitors which I guess nobody uses > anymore. Can't we just get rid of it? This patch fixes my crash problem. Thanks a lot guys! > Signed-off-by: Jean Delvare <[EMAIL PROTECTED]> Acked-by: Michael Buesch <[EMAIL PROTECTED]> > --- > drivers/video/fb_ddc.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > --- linux-2.6.24-rc3.orig/drivers/video/fb_ddc.c 2007-11-17 > 20:23:03.000000000 +0100 > +++ linux-2.6.24-rc3/drivers/video/fb_ddc.c 2007-11-18 12:49:14.000000000 > +0100 > @@ -56,13 +56,12 @@ unsigned char *fb_ddc_read(struct i2c_ad > int i, j; > > algo_data->setscl(algo_data->data, 1); > - algo_data->setscl(algo_data->data, 0); > > for (i = 0; i < 3; i++) { > /* For some old monitors we need the > * following process to initialize/stop DDC > */ > - algo_data->setsda(algo_data->data, 0); > + algo_data->setsda(algo_data->data, 1); > msleep(13); > > algo_data->setscl(algo_data->data, 1); > @@ -97,14 +96,15 @@ unsigned char *fb_ddc_read(struct i2c_ad > algo_data->setsda(algo_data->data, 1); > msleep(15); > algo_data->setscl(algo_data->data, 0); > + algo_data->setsda(algo_data->data, 0); > if (edid) > break; > } > /* Release the DDC lines when done or the Apple Cinema HD display > * will switch off > */ > - algo_data->setsda(algo_data->data, 0); > - algo_data->setscl(algo_data->data, 0); > + algo_data->setsda(algo_data->data, 1); > + algo_data->setscl(algo_data->data, 1); > > return edid; > } > > > > > -- Greetings Michael. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
