lkml.org 
[lkml]   [2007]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: radeonfb i2c regression post-2.6.18.
    Date
    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 <khali@linux-fr.org>
    > To: linux-fbdev-devel@lists.sourceforge.net
    > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>, Antonino Daplas
    > <adaplas@gmail.com>
    > 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 <khali@linux-fr.org>

    Acked-by: Michael Buesch <mb@bu3sch.de>

    > ---
    > 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 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/

    \
     
     \ /
      Last update: 2007-11-23 17:05    [W:0.045 / U:30.820 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site