Re: [PATCH v2 0/6] drm: byteorder fixes
On Fri, Jan 11, 2019 at 2:08 PM Ilia Mirkin wrote: > > > > On Fri, Jan 11, 2019, 05:26 Daniel Vetter > >> On Fri, Jan 11, 2019 at 10:43:42AM +0100, Gerd Hoffmann wrote: >> > On Fri, Jan 11, 2019 at 10:11:09AM +0100, Daniel Vetter wrote: >> > > On Fri, Jan 11, 2019 at 10:08 AM Gerd Hoffmann wrote: >> > > > >> > > > On Fri, Jan 11, 2019 at 01:17:47AM -0500, Ilia Mirkin wrote: >> > > > > Hi Gerd, >> > > > > >> > > > > What happened with this series (and the next one)? >> > > > >> > > > Landed upstream in 4.20. > > > Huh. I must have been looking at an option tree. > >> > > > >> > > > > Semi-relatedly, I wonder if it wouldn't be better to just dump the >> > > > > BIG_ENDIAN flag and just define the "host" format to be the right one >> > > > > for a consistent little-endian interpretation. >> > > > >> > > > Not fully sure what you mean here. >> > > > >> > > > The big endian flag is needed to specify RGB565 or XRGB1555 format in >> > > > big endian byte order. For 32 bit depth we don't need it because >> > > > XRGB big endian == BGRX little endian (see also >> > > > include/drm/drm_fourcc.h). > > > Bleargh. I was thinking xrgb1555 le == bgrx5551 be, but that's clearly false. > Made sense to me last night though. That makes my suggestion considerably > less appealing. > >> > > >> > > Yeah we'd need to add a few more fourcc codes for the missing >> > > big-endian versions. >> > >> > If we do that then yes we can drop the BIG_ENDIAN flag. >> > >> > Not sure what the userspace API implications are, >> > ADDFB2 ioctl comes to mind. >> > >> > > Aside from that I like Ilia's idea of removing >> > > the BIG_ENDIAN flag, it causes lots of aliasing among formats, doesn't >> > > change anything with others, and we probably have a huge mess already >> > > ... >> > >> > Shouldn't be too messy. The place where the DRM_FORMAT_HOST_* formats >> > are defined (include/drm/drm_fourcc.h) is almost the only place where >> > DRM_FORMAT_BIG_ENDIAN is used, and for the DRM_FORMAT_HOST_* users the >> > switch should be transparent. >> >> Yeah we'd need to review current userspace, but I suspect the overlap of >> "uses addfb2" and "runs on BE platform" is 0. > > > Most importantly, "uses bigendian flag" is 0 since we would previously error > out on such modesets. > > Although the need to double up on the 16bpp formats makes this not really > worth it. Ohhh well. I think even with a few formats doubled up it'll be worth it, since we remove all the confusion around the formats where the BE flag is not needed/confusion/causes aliasing with another format. Also given that even ppc is switching to LE I think making LE the forced default assumption for everything will work much better long term. At least with the explicit fourcc code, legacy addfb will pick native endianess (at least since 4.20). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] drm: byteorder fixes
On Fri, Jan 11, 2019, 05:26 Daniel Vetter On Fri, Jan 11, 2019 at 10:43:42AM +0100, Gerd Hoffmann wrote: > > On Fri, Jan 11, 2019 at 10:11:09AM +0100, Daniel Vetter wrote: > > > On Fri, Jan 11, 2019 at 10:08 AM Gerd Hoffmann > wrote: > > > > > > > > On Fri, Jan 11, 2019 at 01:17:47AM -0500, Ilia Mirkin wrote: > > > > > Hi Gerd, > > > > > > > > > > What happened with this series (and the next one)? > > > > > > > > Landed upstream in 4.20. > Huh. I must have been looking at an option tree. > > > > > > > > Semi-relatedly, I wonder if it wouldn't be better to just dump the > > > > > BIG_ENDIAN flag and just define the "host" format to be the right > one > > > > > for a consistent little-endian interpretation. > > > > > > > > Not fully sure what you mean here. > > > > > > > > The big endian flag is needed to specify RGB565 or XRGB1555 format in > > > > big endian byte order. For 32 bit depth we don't need it because > > > > XRGB big endian == BGRX little endian (see also > > > > include/drm/drm_fourcc.h). > Bleargh. I was thinking xrgb1555 le == bgrx5551 be, but that's clearly false. Made sense to me last night though. That makes my suggestion considerably less appealing. > > > > > Yeah we'd need to add a few more fourcc codes for the missing > > > big-endian versions. > > > > If we do that then yes we can drop the BIG_ENDIAN flag. > > > > Not sure what the userspace API implications are, > > ADDFB2 ioctl comes to mind. > > > > > Aside from that I like Ilia's idea of removing > > > the BIG_ENDIAN flag, it causes lots of aliasing among formats, doesn't > > > change anything with others, and we probably have a huge mess already > > > ... > > > > Shouldn't be too messy. The place where the DRM_FORMAT_HOST_* formats > > are defined (include/drm/drm_fourcc.h) is almost the only place where > > DRM_FORMAT_BIG_ENDIAN is used, and for the DRM_FORMAT_HOST_* users the > > switch should be transparent. > > Yeah we'd need to review current userspace, but I suspect the overlap of > "uses addfb2" and "runs on BE platform" is 0. > Most importantly, "uses bigendian flag" is 0 since we would previously error out on such modesets. Although the need to double up on the 16bpp formats makes this not really worth it. Ohhh well. -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] drm: byteorder fixes
On Fri, Jan 11, 2019 at 10:43:42AM +0100, Gerd Hoffmann wrote: > On Fri, Jan 11, 2019 at 10:11:09AM +0100, Daniel Vetter wrote: > > On Fri, Jan 11, 2019 at 10:08 AM Gerd Hoffmann wrote: > > > > > > On Fri, Jan 11, 2019 at 01:17:47AM -0500, Ilia Mirkin wrote: > > > > Hi Gerd, > > > > > > > > What happened with this series (and the next one)? > > > > > > Landed upstream in 4.20. > > > > > > > Semi-relatedly, I wonder if it wouldn't be better to just dump the > > > > BIG_ENDIAN flag and just define the "host" format to be the right one > > > > for a consistent little-endian interpretation. > > > > > > Not fully sure what you mean here. > > > > > > The big endian flag is needed to specify RGB565 or XRGB1555 format in > > > big endian byte order. For 32 bit depth we don't need it because > > > XRGB big endian == BGRX little endian (see also > > > include/drm/drm_fourcc.h). > > > > Yeah we'd need to add a few more fourcc codes for the missing > > big-endian versions. > > If we do that then yes we can drop the BIG_ENDIAN flag. > > Not sure what the userspace API implications are, > ADDFB2 ioctl comes to mind. > > > Aside from that I like Ilia's idea of removing > > the BIG_ENDIAN flag, it causes lots of aliasing among formats, doesn't > > change anything with others, and we probably have a huge mess already > > ... > > Shouldn't be too messy. The place where the DRM_FORMAT_HOST_* formats > are defined (include/drm/drm_fourcc.h) is almost the only place where > DRM_FORMAT_BIG_ENDIAN is used, and for the DRM_FORMAT_HOST_* users the > switch should be transparent. Yeah we'd need to review current userspace, but I suspect the overlap of "uses addfb2" and "runs on BE platform" is 0. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] drm: byteorder fixes
On Fri, Jan 11, 2019 at 10:11:09AM +0100, Daniel Vetter wrote: > On Fri, Jan 11, 2019 at 10:08 AM Gerd Hoffmann wrote: > > > > On Fri, Jan 11, 2019 at 01:17:47AM -0500, Ilia Mirkin wrote: > > > Hi Gerd, > > > > > > What happened with this series (and the next one)? > > > > Landed upstream in 4.20. > > > > > Semi-relatedly, I wonder if it wouldn't be better to just dump the > > > BIG_ENDIAN flag and just define the "host" format to be the right one > > > for a consistent little-endian interpretation. > > > > Not fully sure what you mean here. > > > > The big endian flag is needed to specify RGB565 or XRGB1555 format in > > big endian byte order. For 32 bit depth we don't need it because > > XRGB big endian == BGRX little endian (see also > > include/drm/drm_fourcc.h). > > Yeah we'd need to add a few more fourcc codes for the missing > big-endian versions. If we do that then yes we can drop the BIG_ENDIAN flag. Not sure what the userspace API implications are, ADDFB2 ioctl comes to mind. > Aside from that I like Ilia's idea of removing > the BIG_ENDIAN flag, it causes lots of aliasing among formats, doesn't > change anything with others, and we probably have a huge mess already > ... Shouldn't be too messy. The place where the DRM_FORMAT_HOST_* formats are defined (include/drm/drm_fourcc.h) is almost the only place where DRM_FORMAT_BIG_ENDIAN is used, and for the DRM_FORMAT_HOST_* users the switch should be transparent. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] drm: byteorder fixes
On Fri, Jan 11, 2019 at 10:08 AM Gerd Hoffmann wrote: > > On Fri, Jan 11, 2019 at 01:17:47AM -0500, Ilia Mirkin wrote: > > Hi Gerd, > > > > What happened with this series (and the next one)? > > Landed upstream in 4.20. > > > Semi-relatedly, I wonder if it wouldn't be better to just dump the > > BIG_ENDIAN flag and just define the "host" format to be the right one > > for a consistent little-endian interpretation. > > Not fully sure what you mean here. > > The big endian flag is needed to specify RGB565 or XRGB1555 format in > big endian byte order. For 32 bit depth we don't need it because > XRGB big endian == BGRX little endian (see also > include/drm/drm_fourcc.h). Yeah we'd need to add a few more fourcc codes for the missing big-endian versions. Aside from that I like Ilia's idea of removing the BIG_ENDIAN flag, it causes lots of aliasing among formats, doesn't change anything with others, and we probably have a huge mess already ... -Daniel > > cheers, > Gerd > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] drm: byteorder fixes
On Fri, Jan 11, 2019 at 01:17:47AM -0500, Ilia Mirkin wrote: > Hi Gerd, > > What happened with this series (and the next one)? Landed upstream in 4.20. > Semi-relatedly, I wonder if it wouldn't be better to just dump the > BIG_ENDIAN flag and just define the "host" format to be the right one > for a consistent little-endian interpretation. Not fully sure what you mean here. The big endian flag is needed to specify RGB565 or XRGB1555 format in big endian byte order. For 32 bit depth we don't need it because XRGB big endian == BGRX little endian (see also include/drm/drm_fourcc.h). cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] drm: byteorder fixes
Hi Gerd, What happened with this series (and the next one)? Semi-relatedly, I wonder if it wouldn't be better to just dump the BIG_ENDIAN flag and just define the "host" format to be the right one for a consistent little-endian interpretation. Cheers, -ilia On Wed, Sep 5, 2018 at 2:04 AM Gerd Hoffmann wrote: > > Patch series adds some convinience #defines for host byteoder drm > formats. It fixes drm_mode_addfb() behavior on bigendian machines. For > bug compatibility reasons a mode_config quirk activates the fix. > bochs and virtio-gpu drivers are updated to use the new #defines, set > the new driver feature flag and fix some issues. > > Gerd Hoffmann (6): > drm: replace DRIVER_PREFER_XBGR_30BPP driver flag with mode_config > quirk > drm: byteorder: add DRM_FORMAT_HOST_* > drm: do not mask out DRM_FORMAT_BIG_ENDIAN > drm: fix drm_mode_addfb() on big endian machines. > drm/bochs: fix DRM_FORMAT_* handling for big endian machines. > drm/virtio: fix DRM_FORMAT_* handling > > include/drm/drm_drv.h| 1 - > include/drm/drm_fourcc.h | 22 + > include/drm/drm_mode_config.h| 15 + > drivers/gpu/drm/bochs/bochs_fbdev.c | 5 ++- > drivers/gpu/drm/bochs/bochs_kms.c| 34 +++- > drivers/gpu/drm/bochs/bochs_mm.c | 2 +- > drivers/gpu/drm/drm_framebuffer.c| 17 -- > drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- > drivers/gpu/drm/virtio/virtgpu_display.c | 5 +++ > drivers/gpu/drm/virtio/virtgpu_fb.c | 2 +- > drivers/gpu/drm/virtio/virtgpu_gem.c | 7 +++-- > drivers/gpu/drm/virtio/virtgpu_plane.c | 54 > ++-- > 12 files changed, 101 insertions(+), 65 deletions(-) > > -- > 2.9.3 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
