Small clarification:<br><br><div class="gmail_quote">On Thu, Aug 23, 2012 at 4:23 PM, Daniel Vetter <span dir="ltr"><<a href="mailto:daniel@ffwll.ch" target="_blank">daniel@ffwll.ch</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Thu, Aug 23, 2012 at 07:06:53AM -0700, Shirish S wrote:<br>
> Hello Daniel,<br>
><br>
> On Thu, Aug 23, 2012 at 1:54 AM, Daniel Vetter <<a href="mailto:daniel@ffwll.ch">daniel@ffwll.ch</a>> wrote:<br>
><br>
> > On Wed, Aug 22, 2012 at 10:52:26AM +0300, Ville Syrjälä wrote:<br>
> > > On Tue, Aug 21, 2012 at 04:28:20PM -0700, Shirish S wrote:<br>
> > > > On Tue, Aug 21, 2012 at 7:56 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 06:55:53AM -0700, Shirish S wrote:<br>
> > > > Here are my earlier comments on Jean's patch:<br>
> > > > ><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>
> > > > ><br>
> > > > ><br>
> > > > If i am not wrong am doing exactly what you have said in you comments.<br>
> > > ><br>
> > > > This seems a bit wrong to me. The spec says that the ack for the<br>
> > > > segment address is "don't care", but for the segment pointer the ack is<br>
> > > > required (when segment != 0).<br>
> > > > The variable segFlags is "dont care for block 0 and 1 wheras".<br>
> > > ><br>
> > > > With I2C_M_IGNORE_NAK we would in fact end up reading segment 0 from a<br>
> > > > non E-DDC display, if we try to read segment != 0 from it. Of course<br>
> > > > we shouldn't do that unless the display lied to us about what extension<br>
> > > > blocks it provides.<br>
> > > ><br>
> > > > So I'm not sure if it would be better to trust that the display never<br>
> > > > lies about the extension blocks, or if we should just assume all E-DDC<br>
> > > > displays ack both segment addr and pointer. The no-ack feature seems<br>
> > > > to there for backwards compatibility, for cases where the host always<br>
> > > > sends the segment addr/pointer even when reading segment 0 (which your<br>
> > > > code doesn't do).<br>
> > > ><br>
> > > > To handle it exactly as the spec says, I2C_M_IGNORE_NAK should be split<br>
> > > > into two flags (one for addr, other for data).<br>
> > > ><br>
> > > > Hence i have split the i2c_msg into 3, segment pointer,offset(addr)<br>
> > > > and data pointer.<br>
> > ><br>
> > > I was referring to the addr and data phases of the segment pointer.<br>
> > > According to the spec the ack for the addr is always optional. But I<br>
> > > suppose no sane device would nak the addr, while acking the data.<br>
> ><br>
> > We've seen those. Really.<br>
> ><br>
> > Which is why the current i915 gmbus driver has a hack to never return a<br>
> > NaK on the first i2c transfer. I guess we should fix this by properly<br>
> > supporting the INGNORE_NAK field in our gmbus driver, and setting that on<br>
> > the addr transfer field, too.<br>
> ><br>
> > Thanks for the comment, so are you ok with the current logic?<br>
><br>
><br>
> > I concure with Ville that sending the segment i2c message only when we<br>
> > actually need it, and not unconditionally. DDC is way to broken and<br>
> > claiming that the spec says otherwise doesn't fix all the existing bad hw.<br>
> ><br>
><br>
> Agreed, so do you want me to post another patch, in which i add a function<br>
> only<br>
> if the number of blocks is more than 2?<br>
> Also i had some mistake in the patch set 1, hence i updated it.<br>
<br>
</div></div>I think adding the IGNORE_NAK on the addr i2c transaction would help<br>
unconditionally - like I've said we've seen monitors that suggest that<br>
this is required. And yeah, I think we should send the E-DDC segment<br>
number only if the basic edid block indicates that more than 2 blocks are<br>
availble (and again with IGNORE_NAK, just for paranoia's sake).<br>
<br>
<br>
> Kindly have a look!<br>
<br>
Will do.<br>
<br></blockquote><div><br></div><div>The patch set 2 is based on the 2 comments i received</div><div>I shall post patch set 3 today,incorporating your comments. </div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Yours, Daniel<br>
<div class="HOEnZb"><div class="h5">--<br>
Daniel Vetter<br>
Mail: <a href="mailto:daniel@ffwll.ch">daniel@ffwll.ch</a><br>
Mobile: +41 (0)79 365 57 48<br></div></div></blockquote><div> </div></div>Thanks & Regards,<div>Shirish S</div>