From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm: fourcc byteorder: brings header file comments in line with reality. Date: Tue, 25 Apr 2017 13:26:07 +0300 Message-ID: <20170425102607.GL30290@intel.com> 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> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Michel =?iso-8859-1?Q?D=E4nzer?= Cc: open list , dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, Daniel Vetter , Christian =?iso-8859-1?Q?K=F6nig?= , Gerd Hoffmann List-Id: amd-gfx.lists.freedesktop.org T24gVHVlLCBBcHIgMjUsIDIwMTcgYXQgMTA6MTI6MzdBTSArMDkwMCwgTWljaGVsIETDpG56ZXIg d3JvdGU6Cj4gT24gMjQvMDQvMTcgMTA6MDMgUE0sIFZpbGxlIFN5cmrDpGzDpCB3cm90ZToKPiA+ IE9uIE1vbiwgQXByIDI0LCAyMDE3IGF0IDAzOjU3OjAyUE0gKzA5MDAsIE1pY2hlbCBEw6RuemVy IHdyb3RlOgo+ID4+IE9uIDIyLzA0LzE3IDA3OjA1IFBNLCBWaWxsZSBTeXJqw6Rsw6Qgd3JvdGU6 Cj4gPj4+IE9uIEZyaSwgQXByIDIxLCAyMDE3IGF0IDA2OjE0OjMxUE0gKzAyMDAsIEdlcmQgSG9m Zm1hbm4gd3JvdGU6Cj4gPj4+PiAgIEhpLAo+ID4+Pj4KPiA+Pj4+Pj4gTXkgcGVyc29uYWwgb3Bp bmlvbiBpcyB0aGF0IGZvcm1hdHMgaW4gZHJtX2ZvdXJjYy5oIHNob3VsZCBiZSAKPiA+Pj4+Pj4g aW5kZXBlbmRlbnQgb2YgdGhlIENQVSBieXRlIG9yZGVyIGFuZCB0aGUgZnVuY3Rpb24gCj4gPj4+ Pj4+IGRybV9tb2RlX2xlZ2FjeV9mYl9mb3JtYXQoKSBhbmQgZHJpdmVycyBkZXBlbmRpbmcgb24g dGhhdCBpbmNvcnJlY3QgCj4gPj4+Pj4+IGFzc3VtcHRpb24gYmUgZml4ZWQgaW5zdGVhZC4KPiA+ Pj4+Pgo+ID4+Pj4+IFRoZSBwcm9ibGVtIGlzIHRoaXMgaXNuJ3QgYSBrZXJuZWwtaW50ZXJuYWwg dGhpbmcgYW55IG1vcmUuICBXaXRoIHRoZQo+ID4+Pj4+IGFkZGl0aW9uIG9mIHRoZSBBRERGQjIg aW9jdGwgdGhlIGZvdXJjYyBjb2RlcyBiZWNhbWUgcGFydCBvZiB0aGUKPiA+Pj4+PiBrZXJuZWwv dXNlcnNwYWNlIGFiaSAuLi4KPiA+Pj4+Cj4gPj4+PiBPaywgYWRkZWQgc29tZSBwcmludGsncyB0 byB0aGUgQURERkIgYW5kIEFEREZCMiBjb2RlIHBhdGhzIGFuZCB0ZXN0ZWQgYQo+ID4+Pj4gYml0 LiAgQXBwYXJlbnRseSBwcmV0dHkgbXVjaCBhbGwgdXNlcnNwYWNlIHN0aWxsIHVzZXMgdGhlIEFE REZCIGlvY3RsLgo+ID4+Pj4geG9yZyAobW9kZXNldHRpbmcgZHJpdmVyKSBkb2VzLiAgZ25vbWUt c2hlbGwgaW4gd2F5bGFuZCBtb2RlIGRvZXMuCj4gPj4+PiBTZWVtcyB0aGUgYmlnIHRyYW5zaXRp b24gdG8gQURERkIyIGRpZG4ndCBoYXBwZW4geWV0Lgo+ID4+Pj4KPiA+Pj4+IEkgZ3Vlc3MgdGhh dCBtYWtlcyBjaGFuZ2luZyBkcm1fbW9kZV9sZWdhY3lfZmJfZm9ybWF0ICsgZHJpdmVycyBhCj4g Pj4+PiByZWFzb25hYmxlIG9wdGlvbiAuLi4KPiA+Pj4KPiA+Pj4gWWVhaCwgSSBjYW1lIHRvIHRo ZSBzYW1lIGNvbmNsdXNpb24gYWZ0ZXIgY2hhdHRpbmcgd2l0aCBzb21lCj4gPj4+IGZvbGtzIG9u IGlyYy4KPiA+Pj4KPiA+Pj4gU28gbXkgY3VycmVudCBpZGVhIGlzIHRoYXQgd2UgY2hhbmdlIGFu eSBkcml2ZXIgdGhhdCB3YW50cyB0byBmb2xsb3cgdGhlCj4gPj4+IENQVSBlbmRpYW5uZXNzCj4g Pj4KPiA+PiBUaGlzIGlzbid0IHJlYWxseSBvcHRpb25hbCBmb3IgdmFyaW91cyByZWFzb25zLCBz b21lIG9mIHdoaWNoIGhhdmUgYmVlbgo+ID4+IGNvdmVyZWQgaW4gdGhpcyBkaXNjdXNzaW9uLgo+ ID4+Cj4gPj4KPiA+Pj4gdG8gZGVjbGFyZSBzdXBwb3J0IGZvciBiaWcgZW5kaWFuIGZvcm1hdHMg aWYgdGhlIENQVSBpcwo+ID4+PiBiaWcgZW5kaWFuLiBQcmVzdW1hYmx5IHRoZXNlIGFyZSBtb3N0 bHkgdGhlIHZpcnR1YWwgR1BVIGRyaXZlcnMuCj4gPj4+Cj4gPj4+IEFkZGl0b25hbGx5IHdlJ2xs IG1ha2UgdGhlIG1hcHBpbmcgcGVyZm9ybWVkIGJ5IGRybV9tb2RlX2xlZ2FjeV9mYl9mb3JtYXQo KQo+ID4+PiBkcml2ZXIgY29udHJvbGxlZC4gVGhhdCB3YXkgZHJpdmVycyB0aGF0IGdvdCBjaGFu Z2VkIHRvIGZvbGxvdyBDUFUKPiA+Pj4gZW5kaWFubmVzcyBjYW4gcmV0dXJuIGEgZnJhbWVidWZm ZXIgdGhhdCBtYXRjaGVzIENQVSBlbmRpYW5uZXNzLiBBbmQKPiA+Pj4gZHJpdmVycyB0aGF0IGV4 cGVjdCB0aGUgR1BVIGVuZGlhbm5lc3MgdG8gbm90IGRlcGVuZCBvbiB0aGUgQ1BVCj4gPj4+IGVu ZGlhbm5lc3Mgd2lsbCBrZWVwIHdvcmtpbmcgYXMgdGhleSBkbyBub3cuIFRoZSBkb3duc2lkZSBp cyB0aGF0IHVzZXJzCj4gPj4+IG9mIHRoZSBsZWdhY3kgYWRkZmIgaW9jdGwgd2lsbCBuZWVkIHRv IG1hZ2ljYWxseSBrbm93IHdoaWNoIGVuZGlhbm5lc3MKPiA+Pj4gdGhleSB3aWxsIGdldCwgYnV0 IHRoYXQgaXMgYXBwYXJlbnRseSBhbHJlYWR5IHRoZSBjYXNlLiBBbmQgdXNlcnMgb2YKPiA+Pj4g YWRkZmIyIHdpbGwga2VlcCBvbiBzcGVjaWZ5aW5nIHRoZSBlbmRpYW5uZXNzIGV4cGxpY2l0bHkg d2l0aAo+ID4+PiBEUk1fRk9STUFUX0JJR19FTkRJQU4gdnMuIDAuCj4gPj4KPiA+PiBJJ20gYWZy YWlkIGl0J3Mgbm90IHRoYXQgc2ltcGxlLgo+ID4+Cj4gPj4gVGhlIGRpc3BsYXkgaGFyZHdhcmUg b2Ygb2xkZXIgKHByZS1SNjAwIGdlbmVyYXRpb24pIFJhZGVvbiBHUFVzIGRvZXMgbm90Cj4gPj4g c3VwcG9ydCB0aGUgImJpZyBlbmRpYW4iIGZvcm1hdHMgZGlyZWN0bHkuIEluIG9yZGVyIHRvIGFs bG93IHVzZXJzcGFjZQo+ID4+IHRvIGFjY2VzcyBwaXhlbCBkYXRhIGluIG5hdGl2ZSBlbmRpYW5u ZXNzIHdpdGggdGhlIENQVSwgd2UgaW5zdGVhZCB1c2UKPiA+PiBieXRlLXN3YXBwaW5nIGZ1bmN0 aW9uYWxpdHkgd2hpY2ggb25seSBhZmZlY3RzIENQVSBhY2Nlc3MuCj4gPiAKPiA+IE9LLCBJJ20g Z2V0dGluZyBjb25mdXNlZC4gQmFzZWQgb24gb3VyIGlyYyBkaXNjdXNzaW9uIEkgZ290IHRoZQo+ ID4gaW1wcmVzc2lvbiB5b3UgZG9uJ3QgYnl0ZSBzd2FwIENQVSBhY2Nlc3Nlcy4KPiAKPiBTb3Jy eSBmb3IgdGhlIGNvbmZ1c2lvbi4gVGhlIHJhZGVvbiBrZXJuZWwgZHJpdmVyIGRvZXMgc3VwcG9y dAo+IGJ5dGUtc3dhcHBpbmcgZm9yIENQVSBhY2Nlc3MgdG8gVlJBTSB3aXRoIHByZS1SNjAwIEdQ VXMsIGFuZCB0aGlzIGlzCj4gdXNlZCBmb3IgZmJkZXYgZW11bGF0aW9uLiBXaGF0IEkgbWVhbnQg b24gSVJDIGlzIHRoYXQgdGhlIHhmODYtdmlkZW8tYXRpCj4gcmFkZW9uIGRyaXZlciBkb2Vzbid0 IG1ha2UgdXNlIG9mIHRoaXMsIG1vc3RseSBiZWNhdXNlIGl0IG9ubHkgYXBwbGllcwo+IHdoaWxl IGEgQk8gaXMgaW4gVlJBTSwgYW5kIHVzZXJzcGFjZSBjYW4ndCBjb250cm9sIHdoZW4gdGhhdCdz IHRoZSBjYXNlCj4gKHdoaWxlIGEgQk8gaXNuJ3QgYmVpbmcgc2Nhbm5lZCBvdXQpLgoKU28gdGhh dCB3YXMgbXkgb3RoZXIgcXVlc3Rpb24uIFNvIGlmIHNvbWVvbmUganVzdCBjcmVhdGVzIGEgYm8s IEkgcHJlc3VtZQp0dG0gY2FuIG1vcmUgb3IgbGVzcyBtb3ZlIGl0IGJldHdlZW4gc3lzdGVtIG1l bW9yeSBhbmQgdnJhbSBhdCBhbnkKdGltZS4gU28gaWYgd2UgdGhlbiBtbWFwIHRoZSBibywgZG9l cyBpdCBtZWFuIHRoZSBDUFUgd2lsbCBzZWUgdGhlIGJ5dGVzCmluIGRpZmZlcmVudCBvcmRlciBk ZXBlbmRpbmcgb24gd2hlcmUgdGhlIGJvIGhhcHBlbnMgdG8gbGl2ZSBhdAp0aGUgdGltZSB0aGUg Q1BVIGFjY2VzcyBoYXBwZW5zPwoKQW5kIGhvdyB3b3VsZCB0aGF0IHdvcmsgd2loIGR1bWIgYm9z PyBXb3VsZCB0aGV5IGJlIGZvcmNlZCB0byBsaXZlIGluIHZyYW0/Ckkgc2VlIGl0J3MgcGFzc2lu ZyBWUkFNIGFzIHRoZSBpbml0aWFsIGRvbWFpbiwgYnV0IEkgY2FuJ3QgcXVpY2tseSBzZWUKd2hl dGhlciB0aGF0IHdvdWxkIG1lYW4gaXQgY2FuJ3QgZXZlbiBiZSBtb3ZlZCBvdXQuCgo+IAo+IAo+ ID4gQnV0IHNpbmNlIHlvdSBkbywgaG93IGRvIHlvdSBkZWFsIHdpdGggbWl4aW5nIDhicHAgdnMu IDE2YnBwIHZzLiAzMmJwcD8KPiAKPiBUaGUgYnl0ZS1zd2FwcGluZyBpcyBjb25maWd1cmVkIHBl ci1CTyB2aWEgdGhlCj4gUkFERU9OX1RJTElOR19TV0FQXzE2LzMyQklUIGZsYWdzLgoKV2hpY2gg dHJhbnNsYXRlcyBpbnRvIHVzYWdlIG9mIHRoZSBzdXJmYWNlIHJlZ3MgaXQgc2VlbXMuIFNvIEkg d2Fzbid0CnRvdGFsbHkgY3JhenkgdG8gdGhpbmsgdGhhdCBzdWNoIHRoaW5ncyBleGlzdGVkIDop CgotLSAKVmlsbGUgU3lyasOkbMOkCkludGVsIE9UQwpfX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBs aXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1h bi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1429448AbdDYK0V (ORCPT ); Tue, 25 Apr 2017 06:26:21 -0400 Received: from mga07.intel.com ([134.134.136.100]:51983 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1164894AbdDYK0M (ORCPT ); Tue, 25 Apr 2017 06:26:12 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,249,1488873600"; d="scan'208";a="78577887" Date: Tue, 25 Apr 2017 13:26:07 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Michel =?iso-8859-1?Q?D=E4nzer?= Cc: amd-gfx@lists.freedesktop.org, open list , dri-devel@lists.freedesktop.org, Gerd Hoffmann , Daniel Vetter , Christian =?iso-8859-1?Q?K=F6nig?= Subject: Re: [PATCH] drm: fourcc byteorder: brings header file comments in line with reality. Message-ID: <20170425102607.GL30290@intel.com> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? And how would that work wih dumb bos? Would they be forced to live in vram? I see it's passing VRAM as the initial domain, but I can't quickly see whether that would mean it can't even be moved out. > > > > But since you do, how do you deal with mixing 8bpp vs. 16bpp vs. 32bpp? > > The byte-swapping is configured per-BO via the > RADEON_TILING_SWAP_16/32BIT flags. Which translates into usage of the surface regs it seems. So I wasn't totally crazy to think that such things existed :) -- Ville Syrjälä Intel OTC