Hello Daniel,<br><br><div class="gmail_quote">On Thu, Aug 23, 2012 at 1:54 AM, 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 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>
> > > <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>
</div></div>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></blockquote><div>Thanks for the comment, so are you ok with the current logic?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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></blockquote><div><br></div><div>Agreed, so do you want me to post another patch, in which i add a function only</div><div>if the number of blocks is more than 2?</div>
<div>Also i had some mistake in the patch set 1, hence i updated it.</div><div>Kindly have a look!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888">-Daniel<br>
--<br>
Daniel Vetter<br>
Mail: <a href="mailto:daniel@ffwll.ch">daniel@ffwll.ch</a><br>
Mobile: +41 (0)79 365 57 48<br>
</font></span></blockquote></div>Thanks & Regards,<div>Shirish S</div>