DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com A7544341450
Message-ID: <1492780323.25675.45.camel@redhat.com>
Subject: Re: [PATCH] drm: fourcc byteorder: brings header file comments in
 line with reality.
From: Gerd Hoffmann <kraxel@redhat.com>
To: Christian =?ISO-8859-1?Q?K=F6nig?= <christian.koenig@amd.com>
Cc: Ville =?ISO-8859-1?Q?Syrj=E4l=E4?= 
        <ville.syrjala@linux.intel.com>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        David Airlie <airlied@linux.ie>,
        Michel =?ISO-8859-1?Q?D=E4nzer?= <michel@daenzer.net>,
        open list <linux-kernel@vger.kernel.org>,
        dri-devel@lists.freedesktop.org, Pekka Paalanen <ppaalanen@gmail.com>,
        Sean Paul <seanpaul@chromium.org>, amd-gfx@lists.freedesktop.org,
        Alex Deucher <alexdeucher@gmail.com>,
        Daniel Vetter <daniel.vetter@intel.com>,
        Ilia Mirkin <imirkin@alum.mit.edu>
Date: Fri, 21 Apr 2017 15:12:03 +0200
In-Reply-To: <f1b0ae60-b20e-ad52-7fbf-32f2c045f5fc@amd.com>
References: <20170421075825.6307-1-kraxel@redhat.com>
         <20170421092530.GE30290@intel.com> <1492768218.25675.33.camel@redhat.com>
         <20170421110804.GH30290@intel.com>
         <f1b0ae60-b20e-ad52-7fbf-32f2c045f5fc@amd.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Mime-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2432
Lines: 60

  Hi,

> > "native" to me feels more like "native to the GPU" since these things
> > really are tied to the GPU not the CPU. That's also why I went with the
> > explicit endianness originally so that the driver could properly declare
> > what the GPU supports.
> And to be honest I would really prefer to stick with that approach for 
> exactly that reason.
> 
> The proposed change would require that drivers have different code path 
> for different CPU byte order. Those code path tend to be not tested very 
> well and are additional complexity we probably don't want inside the driver.

We can add fixed-endian #defines without too much effort, at least for
the 8 bits per color formats.  In qemu we have the same problem, only
with pixman.  Those formats are native endian too, but often we have to
handle a fixed format, so we did this:

/*
 * pixman image formats are defined to be native endian,
 * that means host byte order on qemu.  So we go define
 * fixed formats here for cases where it is needed, like
 * feeding libjpeg / libpng and writing screenshots.
 */

#ifdef HOST_WORDS_BIGENDIAN
# define PIXMAN_BE_r8g8b8     PIXMAN_r8g8b8
# define PIXMAN_BE_x8r8g8b8   PIXMAN_x8r8g8b8
# define PIXMAN_BE_a8r8g8b8   PIXMAN_a8r8g8b8
# define PIXMAN_BE_b8g8r8x8   PIXMAN_b8g8r8x8
# define PIXMAN_BE_b8g8r8a8   PIXMAN_b8g8r8a8
# define PIXMAN_BE_r8g8b8x8   PIXMAN_r8g8b8x8
# define PIXMAN_BE_r8g8b8a8   PIXMAN_r8g8b8a8
# define PIXMAN_BE_x8b8g8r8   PIXMAN_x8b8g8r8
# define PIXMAN_BE_a8b8g8r8   PIXMAN_a8b8g8r8
# define PIXMAN_LE_x8r8g8b8   PIXMAN_b8g8r8x8
#else
# define PIXMAN_BE_r8g8b8     PIXMAN_b8g8r8
# define PIXMAN_BE_x8r8g8b8   PIXMAN_b8g8r8x8
# define PIXMAN_BE_a8r8g8b8   PIXMAN_b8g8r8a8
# define PIXMAN_BE_b8g8r8x8   PIXMAN_x8r8g8b8
# define PIXMAN_BE_b8g8r8a8   PIXMAN_a8r8g8b8
# define PIXMAN_BE_r8g8b8x8   PIXMAN_x8b8g8r8
# define PIXMAN_BE_r8g8b8a8   PIXMAN_a8b8g8r8
# define PIXMAN_BE_x8b8g8r8   PIXMAN_r8g8b8x8
# define PIXMAN_BE_a8b8g8r8   PIXMAN_r8g8b8a8
# define PIXMAN_LE_x8r8g8b8   PIXMAN_x8r8g8b8
#endif

> 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 ...

cheers,
  Gerd