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

[Shirish S]

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:
> > Hello Ville Syrjälä,
> >
> > On Tue, Aug 21, 2012 at 4:26 AM, Ville Syrjälä <
> > ville.syrjala at linux.intel.com> wrote:
> >
> > > On Tue, Aug 21, 2012 at 12:40:48PM +0530, Shirish S wrote:
> > > > The current logic for probing ddc is limited to
> > > > 2 blocks (256 bytes), this patch adds support
> > > > for the 4 block (512) data.
> > >
> > > A patch for this was already sent a long time ago:
> > >
> http://lists.freedesktop.org/archives/dri-devel/2011-December/017246.html
> > >
> > > I tried the patch you have mentioned,but its not working in my setup.
> > Also did anyone else test this!!
>
> Not that I know.
>
> > I find that although the author asks the i2c to read for 3 msgs, it
> > verifies only for 2.Kindly correct me if i am wrong.
>
> Yeah, it looks like the return value isn't checked correctly.
>
> > My patch i have verified on the analyser for exynos5 platform.
>
> Your patch always sends the segment which seems a bit pointless, and
> possibly questionable in case a non-EDDC display gets confused by the
> segment information. Jean's patch only sends the segment when needed,
> which feels like the safer option, and it would also avoid increasing
> the amount of i2c traffic.
>
> As per the spec we need to pass  a single 8-bit segment index to the
display via the I2C address 30h
which is DDC_SEGMENT_ADDR, and the ACK is mandatory on the same for block 2
and 3.
(the first of i2c_msg). Although the verification of this is not required
or mandatory for non-EDDC(block 0 and 1),
 i have verified that its presence does not affect non-EDDC displays.(Using
 HDMI analyser-  Agilent N5988A)
Another option to avoid i2c traffic would be to add another function to
probe EDDC blocks,but beg to differ
i dont think current method would affect i2c traffic to an alarming extent.

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.

"a single 8-bit segment index is  passed to the display via the I2C
address 30h(segment pointer).

 Data from the selected segment is then immediately  read via the
regular DDC2 address using a repeated  I2C 'START' signal"

 --
> Ville Syrjälä
> Intel OTC
>

Regards,
Shirish S
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120821/d4ad3942/attachment.html>