lkml.org 
[lkml]   [2012]   [Feb]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] drm/i915: Fix single msg gmbus_xfers writes
    On Mon, Feb 20, 2012 at 07:22:00PM +0800, Daniel Kurtz wrote:
    > On Feb 15, 2012 6:48 PM, "Daniel Vetter" <daniel@ffwll.ch> wrote:
    > >
    > > On Thu, Feb 09, 2012 at 12:03:17PM -0800, Benson Leung wrote:
    > > > gmbus_xfer with a single message (particularly a single message write) would
    > > > set Bus Cycle Select to 100b, the Gen Stop cycle, instead of 101b,
    > > > No Index, Stop cycle. This would not start single message i2c transactions.
    > > >
    > > > Also, gmbus_xfer done: will disable the interface without checking if
    > > > it is idle. In the case of writes, there will be no wait on status or delay
    > > > to ensure the write starts and completes before the interface is turned off.
    > > >
    > > > Fixed the former issue by using the same cycle selection as used in the
    > > > I2C_M_RD for the write case.
    > > > GMBUS_CYCLE_WAIT | (i + 1 == num ? GMBUS_CYCLE_STOP : 0)
    > > > Fixed the latter by waiting on GMBUS_ACTIVE to deassert before disable.
    > > >
    > > > Signed-off-by: Benson Leung <bleung@chromium.org>
    >
    > Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
    >
    > >
    > > Can you clarify the commit message a bit and say that the first hunk is
    > > just for optics and the issue is only with the write path (because the
    > > read path is correct already). Silly me is just to easily confused ;-)
    > >
    > > Btw, I've reworked the gmbus -> gpio bit-banging fallback code a bit and
    > > if that passes review and all I'll reenable gmbus by default again. See
    > >
    > > http://cgit.freedesktop.org/~danvet/drm/log/?h=gmbus
    >
    > If the write case is fixed by Benson's patch, is there any known use
    > case that still requires i2c bit banging on these pins? It would be a
    > nice cleanup to remove it completely.

    Let's first see how much things blow up when re-enabling gmbus again ;-)

    > Also, I can think of at least two further potential performance
    > improvements that I was wondering if anybody has yet pursued:
    > (1) Enabling the i915's gmbus interrupt. This would eliminate the
    > need for the (relatively slow) wait_for polling loop.
    > (2) Taking advantage of the i915's "INDEX" cycles to combine writing
    > a (1 or 2 byte) address & reading back an array of bytes into a single
    > transaction.

    Afaik no one looked into this, but patches are highly welcome.

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


    \
     
     \ /
      Last update: 2012-02-20 14:33    [W:0.028 / U:59.640 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site