Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1951278AbdDZCKm (ORCPT ); Tue, 25 Apr 2017 22:10:42 -0400 Received: from mail.netline.ch ([148.251.143.178]:44275 "EHLO netline-mail3.netline.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1950029AbdDZCKb (ORCPT ); Tue, 25 Apr 2017 22:10:31 -0400 Subject: Re: [PATCH] drm: fourcc byteorder: brings header file comments in line with reality. To: =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= Cc: open list , dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, Daniel Vetter , =?UTF-8?Q?Christian_K=c3=b6nig?= , Gerd Hoffmann References: <20170421092530.GE30290@intel.com> <1492768218.25675.33.camel@redhat.com> <20170421110804.GH30290@intel.com> <1492780323.25675.45.camel@redhat.com> <1492791271.25675.57.camel@redhat.com> <20170422100522.GS30290@intel.com> <20170424130348.GV30290@intel.com> <20170425102607.GL30290@intel.com> From: =?UTF-8?Q?Michel_D=c3=a4nzer?= Message-ID: Date: Wed, 26 Apr 2017 11:10:20 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170425102607.GL30290@intel.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4001 Lines: 84 On 25/04/17 07:26 PM, Ville Syrjälä wrote: > On Tue, Apr 25, 2017 at 10:12:37AM +0900, Michel Dänzer wrote: >> On 24/04/17 10:03 PM, Ville Syrjälä wrote: >>> On Mon, Apr 24, 2017 at 03:57:02PM +0900, Michel Dänzer wrote: >>>> On 22/04/17 07:05 PM, Ville Syrjälä wrote: >>>>> On Fri, Apr 21, 2017 at 06:14:31PM +0200, Gerd Hoffmann wrote: >>>>>> Hi, >>>>>> >>>>>>>> My personal opinion is that formats in drm_fourcc.h should be >>>>>>>> independent of the CPU byte order and the function >>>>>>>> drm_mode_legacy_fb_format() and drivers depending on that incorrect >>>>>>>> assumption be fixed instead. >>>>>>> >>>>>>> The problem is this isn't a kernel-internal thing any more. With the >>>>>>> addition of the ADDFB2 ioctl the fourcc codes became part of the >>>>>>> kernel/userspace abi ... >>>>>> >>>>>> Ok, added some printk's to the ADDFB and ADDFB2 code paths and tested a >>>>>> bit. Apparently pretty much all userspace still uses the ADDFB ioctl. >>>>>> xorg (modesetting driver) does. gnome-shell in wayland mode does. >>>>>> Seems the big transition to ADDFB2 didn't happen yet. >>>>>> >>>>>> I guess that makes changing drm_mode_legacy_fb_format + drivers a >>>>>> reasonable option ... >>>>> >>>>> Yeah, I came to the same conclusion after chatting with some >>>>> folks on irc. >>>>> >>>>> So my current idea is that we change any driver that wants to follow the >>>>> CPU endianness >>>> >>>> This isn't really optional for various reasons, some of which have been >>>> covered in this discussion. >>>> >>>> >>>>> to declare support for big endian formats if the CPU is >>>>> big endian. Presumably these are mostly the virtual GPU drivers. >>>>> >>>>> Additonally we'll make the mapping performed by drm_mode_legacy_fb_format() >>>>> driver controlled. That way drivers that got changed to follow CPU >>>>> endianness can return a framebuffer that matches CPU endianness. And >>>>> drivers that expect the GPU endianness to not depend on the CPU >>>>> endianness will keep working as they do now. The downside is that users >>>>> of the legacy addfb ioctl will need to magically know which endianness >>>>> they will get, but that is apparently already the case. And users of >>>>> addfb2 will keep on specifying the endianness explicitly with >>>>> DRM_FORMAT_BIG_ENDIAN vs. 0. >>>> >>>> I'm afraid it's not that simple. >>>> >>>> The display hardware of older (pre-R600 generation) Radeon GPUs does not >>>> support the "big endian" formats directly. In order to allow userspace >>>> to access pixel data in native endianness with the CPU, we instead use >>>> byte-swapping functionality which only affects CPU access. >>> >>> OK, I'm getting confused. Based on our irc discussion I got the >>> impression you don't byte swap CPU accesses. >> >> Sorry for the confusion. The radeon kernel driver does support >> byte-swapping for CPU access to VRAM with pre-R600 GPUs, and this is >> used for fbdev emulation. What I meant on IRC is that the xf86-video-ati >> radeon driver doesn't make use of this, mostly because it only applies >> while a BO is in VRAM, and userspace can't control when that's the case >> (while a BO isn't being scanned out). > > So that was my other question. So if someone just creates a bo, I presume > ttm can more or less move it between system memory and vram at any > time. So if we then mmap the bo, does it mean the CPU will see the bytes > in different order depending on where the bo happens to live at > the time the CPU access happens? If either of the RADEON_TILING_SWAP_16/32BIT flags was set when the BO was created, yes. That's why the xf86-video-ati radeon driver doesn't use this functionality. > And how would that work wih dumb bos? radeon_mode_dumb_create doesn't set the RADEON_TILING_SWAP_16/32BIT flags, so no byte swapping is performed for dumb BOs even in VRAM. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer