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

[Shirish S]

Small clarification:

On Thu, Aug 23, 2012 at 4:23 PM, Daniel Vetter <daniel at ffwll.ch> wrote:

> On Thu, Aug 23, 2012 at 07:06:53AM -0700, Shirish S wrote:
> > Hello Daniel,
> >
> > On Thu, Aug 23, 2012 at 1:54 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> >
> > > 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.
> > >
> > > Thanks for the comment, so are you ok with the current logic?
> >
> >
> > > 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.
> > >
> >
> > Agreed, so do you want me to post another patch, in which i add a
> function
> > only
> > if the number of blocks is more than 2?
> > Also i had some mistake in the patch set 1, hence i updated it.
>
> I think adding the IGNORE_NAK on the addr i2c transaction would help
> unconditionally - like I've said we've seen monitors that suggest that
> this is required. And yeah, I think we should send the E-DDC segment
> number only if the basic edid block indicates that more than 2 blocks are
> availble (and again with IGNORE_NAK, just for paranoia's sake).
>
>
> > Kindly have a look!
>
> Will do.
>
>
The patch set 2 is based on the 2 comments i received
I shall post patch set 3 today,incorporating your comments.

Yours, Daniel
> --
> Daniel Vetter
> Mail: daniel at ffwll.ch
> Mobile: +41 (0)79 365 57 48
>

Thanks & Regards,
Shirish S
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120823/7e2d5cb9/attachment-0001.html>