[PATCH] drm: edid: add support for E-DDC

[Daniel Vetter]

On Wed, Aug 22, 2012 at 10:52:26AM +0300, Ville Syrjälä wrote:
> On Tue, Aug 21, 2012 at 04:28:20PM -0700, Shirish S wrote:
> > On Tue, Aug 21, 2012 at 7:56 AM, Ville Syrjälä <
> > ville.syrjala at linux.intel.com> wrote:
> > 
> > > On Tue, Aug 21, 2012 at 06:55:53AM -0700, Shirish S wrote:
> > Here are my earlier comments on Jean's patch:
> > > http://lists.freedesktop.org/archives/dri-devel/2012-February/019069.html
> > >
> > >
> >  If i am not wrong am doing exactly what you have said in you comments.
> > 
> > This seems a bit wrong to me. The spec says that the ack for the
> > segment address is "don't care", but for the segment pointer the ack is
> > required (when segment != 0).
> > The variable segFlags is "dont care for block 0 and 1 wheras".
> > 
> > With I2C_M_IGNORE_NAK we would in fact end up reading segment 0 from a
> > non E-DDC display, if we try to read segment != 0 from it. Of course
> > we shouldn't do that unless the display lied to us about what extension
> > blocks it provides.
> > 
> > So I'm not sure if it would be better to trust that the display never
> > lies about the extension blocks, or if we should just assume all E-DDC
> > displays ack both segment addr and pointer. The no-ack feature seems
> > to there for backwards compatibility, for cases where the host always
> > sends the segment addr/pointer even when reading segment 0 (which your
> > code doesn't do).
> > 
> > To handle it exactly as the spec says, I2C_M_IGNORE_NAK should be split
> > into two flags (one for addr, other for data).
> > 
> > Hence i have split the i2c_msg into 3, segment pointer,offset(addr)
> > and data pointer.
> 
> I was referring to the addr and data phases of the segment pointer.
> According to the spec the ack for the addr is always optional. But I
> suppose no sane device would nak the addr, while acking the data.

We've seen those. Really.

Which is why the current i915 gmbus driver has a hack to never return a
NaK on the first i2c transfer. I guess we should fix this by properly
supporting the INGNORE_NAK field in our gmbus driver, and setting that on
the addr transfer field, too.

I concure with Ville that sending the segment i2c message only when we
actually need it, and not unconditionally. DDC is way to broken and
claiming that the spec says otherwise doesn't fix all the existing bad hw.
-Daniel
-- 
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48