<br><br><div class="gmail_quote">On Tue, Aug 21, 2012 at 7:56 AM, Ville Syrjälä <span dir="ltr"><<a href="mailto:ville.syrjala@linux.intel.com" target="_blank">ville.syrjala@linux.intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On Tue, Aug 21, 2012 at 06:55:53AM -0700, Shirish S wrote:<br>
> Hello Ville Syrjälä,<br>
><br>
> On Tue, Aug 21, 2012 at 4:26 AM, Ville Syrjälä <<br>
> <a href="mailto:ville.syrjala@linux.intel.com">ville.syrjala@linux.intel.com</a>> wrote:<br>
><br>
> > On Tue, Aug 21, 2012 at 12:40:48PM +0530, Shirish S wrote:<br>
> > > The current logic for probing ddc is limited to<br>
> > > 2 blocks (256 bytes), this patch adds support<br>
> > > for the 4 block (512) data.<br>
> ><br>
> > A patch for this was already sent a long time ago:<br>
> > <a href="http://lists.freedesktop.org/archives/dri-devel/2011-December/017246.html" target="_blank">http://lists.freedesktop.org/archives/dri-devel/2011-December/017246.html</a><br>
> ><br>
> > I tried the patch you have mentioned,but its not working in my setup.<br>
> Also did anyone else test this!!<br>
<br>
</div>Not that I know.<br>
<div class="im"><br>
> I find that although the author asks the i2c to read for 3 msgs, it<br>
> verifies only for 2.Kindly correct me if i am wrong.<br>
<br>
</div>Yeah, it looks like the return value isn't checked correctly.<br>
<div class="im"><br>
> My patch i have verified on the analyser for exynos5 platform.<br>
<br>
</div>Your patch always sends the segment which seems a bit pointless, and<br>
possibly questionable in case a non-EDDC display gets confused by the<br>
segment information. Jean's patch only sends the segment when needed,<br>
which feels like the safer option, and it would also avoid increasing<br>
the amount of i2c traffic.<br>
<br></blockquote><div>As per the spec we need to pass a single 8-bit segment index to the display via the I2C address 30h</div><div>which is <span style="background-color:rgb(255,255,255);color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px">DDC_SEGMENT_ADDR, and the ACK is mandatory on the same for block 2 and 3.</span></div>
<div><span style="background-color:rgb(255,255,255);color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px">(the first of i2c_msg). Although the verification of this is not </span><span style="background-color:rgb(255,255,255);color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px">required or mandatory for non-EDDC(block 0 and 1),</span></div>
<div><span style="background-color:rgb(255,255,255);color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px"> i have verified that its presence does not affect non-EDDC displays.(Using </span><span style="background-color:rgb(255,255,255);color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px"> </span><span style="background-color:rgb(255,255,255);color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px">HDMI analyser- Agilent N5988A</span><span style="background-color:rgb(255,255,255);color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px">)</span></div>
<div><font color="#222222" face="arial, sans-serif">Another option to avoid i2c traffic would be to add another function to probe EDDC blocks,but beg to differ</font></div><div><font color="#222222" face="arial, sans-serif">i dont think current method would affect i2c traffic to an alarming extent.</font></div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Here are my earlier comments on Jean's patch:<br>
<a href="http://lists.freedesktop.org/archives/dri-devel/2012-February/019069.html" target="_blank">http://lists.freedesktop.org/archives/dri-devel/2012-February/019069.html</a><br>
<div class="HOEnZb"><div class="h5"><br></div></div></blockquote><div> </div><div> If i am not wrong am doing exactly what you have said in you comments.</div><div><pre style="white-space:pre-wrap">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).
<span style="font-family:arial;white-space:normal">The variable segFlags is "dont care for block 0 and 1 wheras".</span>
</pre><pre style="white-space:pre-wrap">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).</pre><pre><pre style="white-space:pre-wrap"><span style="font-family:arial;white-space:normal">Hence i have split the i2c_msg into 3, segment pointer,offset(addr) and data pointer.</span></pre>
<pre><pre><font face="arial"><span style="white-space:normal">"a single 8-bit segment index is passed to the display via the I2C address 30h(segment pointer).</span></font></pre><pre><font face="arial"><span style="white-space:normal"> Data from the selected segment is then immediately read via the regular DDC2 address using a repeated I2C 'START' signal"</span></font></pre>
</pre><span style="white-space:pre-wrap">
</span></pre></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
--<br>
Ville Syrjälä<br>
Intel OTC<br></div></div></blockquote><div><br></div></div>Regards,<div>Shirish S</div>