lkml.org 
[lkml]   [2015]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] drm/vmwgfx: switch from ioremap_cache to memremap
    From
    Date
    On 10/13/2015 08:48 PM, Dan Williams wrote:
    > On Tue, Oct 13, 2015 at 11:37 AM, Thomas Hellstrom
    > <thellstrom@vmware.com> wrote:
    >> On 10/13/2015 06:35 PM, Dan Williams wrote:
    >>> On Mon, Oct 12, 2015 at 10:18 PM, Thomas Hellstrom
    >>> <thellstrom@vmware.com> wrote:
    >>>> Hi!
    >>>>
    >>>> On 10/13/2015 12:35 AM, Dan Williams wrote:
    >>>>> Per commit 2e586a7e017a "drm/vmwgfx: Map the fifo as cached" the driver
    >>>>> expects the fifo registers to be cacheable. In preparation for
    >>>>> deprecating ioremap_cache() convert its usage in vmwgfx to memremap().
    >>>>>
    >>>>> Cc: David Airlie <airlied@linux.ie>
    >>>>> Cc: Thomas Hellstrom <thellstrom@vmware.com>
    >>>>> Cc: Sinclair Yeh <syeh@vmware.com>
    >>>>> Cc: dri-devel@lists.freedesktop.org
    >>>>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
    >>>> While I have nothing against the conversion, what's stopping the
    >>>> compiler from reordering writes on a generic architecture and caching
    >>>> and reordering reads on x86 in particular? At the very least it looks to
    >>>> me like the memory accesses of the memremap'd memory needs to be
    >>>> encapsulated within READ_ONCE and WRITE_ONCE.
    >>> Hmm, currently the code is using ioread32/iowrite32 which only do
    >>> volatile accesses, whereas READ_ONCE / WRITE_ONCE have a memory
    >>> clobber on entry and exit. So, I'm assuming all you need is the
    >>> guarantee of "no compiler re-ordering" and not the stronger
    >>> READ_ONCE/WRITE_ONCE guarantees, but that still seems broken compared
    >>> to explicit fencing where it matters.
    >> I'm not quite sure I follow you here, it looks to me like READ_ONCE()
    >> and WRITE_ONCE() are implemented as
    >> volatile accesses,
    > Ah, sorry, I was looking at the default case...
    >
    >> https://urldefense.proofpoint.com/v2/url?u=http-3A__lxr.free-2Delectrons.com_source_include_linux_compiler.h-23L215&d=BQIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=JRxebmjcR4J-yhD0wROjKrAKyto5OeetIvqt7MqV_WA&s=zn7YmnS74zjM3Sd5Dp9mZnSL27jqel6cwRHwYV6gU3U&e=
    >>
    >> just like ioread32 and iowrite32
    >>
    >> https://urldefense.proofpoint.com/v2/url?u=http-3A__lxr.free-2Delectrons.com_source_include_asm-2Dgeneric_io.h-23L54&d=BQIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=JRxebmjcR4J-yhD0wROjKrAKyto5OeetIvqt7MqV_WA&s=y4dD2GUpcZVHljnThYugF-YLTgeP6En4JwoOnkaLg7A&e=
    >>
    >> which would minimize any potential impact of this change.
    >> IMO optimizing the memory accesses can be done as a later step.
    >>
    > Ok, I'll make local read_fifo() and write_fifo() macros to make this
    > explicit. Are these names ok with you?

    Sure.

    Thanks,
    Thomas



    \
     
     \ /
      Last update: 2015-10-13 21:21    [W:3.228 / U:0.192 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site