On 07/12/16 16:42, Robin Murphy wrote: > On 07/12/16 15:57, Daniel Vetter wrote: >> On Wed, Dec 07, 2016 at 03:31:40PM +0000, Robin Murphy wrote: >>> Under a big-endian kernel, colours on the framebuffer all come out a >>> delightful shade of wrong, since we fail to take the reversed byte >>> order into account. Fortunately, the HDLCD has a control bit to make it >>> automatically byteswap big-endian data; let's use it as appropriate. >>> >>> Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx> >> >> fourcc codes (and the drm fourcc codes) are supposed to be little-endian. >> All of them. So either we fix up the drivers and userspace to set that >> flag correctly (which would mean hdlcd should expose twice as many >> formats, each one with the little and big endian mode). > > AFAICS, SIMPLEFB_FORMATS *is* supposed to be explicitly little-endian > modes. I see that DRM_FORMAT_BIG_ENDIAN exists, but nothing in-tree ever > sets it :/ > > My vague (and probably wrong) assumption was that the HDLCD is still > expecting little-endian data, but the the CPU is writing framebuffer > data as host-endian words, hence what the HDLCD thinks is xRGB is > actually RGBx under a big-endian kernel - It's certainly consistent > between kernel (fbcon) and userspace (fb-test[1]): white is yellow, blue > is black, green is red and red is green. I don't know how to test > "proper" DRM (I've failed to get X to work with the Arch Linux ARM > distro I have on there at the moment). > > Once again I'm somewhat out of my depth here - I just found a thing that > seemed appropriate and visibly resolved the immediate problem :) > By comparison, the same use-cases (fbcon and fb-test) under the same > big-endian kernel do *not* show the same problem with nouveau driving a > PCIe 7600GT card, which led me to believe it was HDLCD-specific. Off the back of that, upon closer inspection, nv_crtc_commit() would appear to already be doing very much the equivalent thing to what I'm doing in this patch, so now I have no idea whether this is right or everything's wrong. Robin. > [1]:https://github.com/prpplague/fb-test-app > >> Or we nuke the big endian flag from drm fourcc. Adding AMD folks who afaik >> are the only other ones who ever cared about big endian in drm. >> -Daniel >> >>> --- >>> drivers/gpu/drm/arm/hdlcd_crtc.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c >>> index 28341b32067f..eceb7bed5dd0 100644 >>> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c >>> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c >>> @@ -63,6 +63,7 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc) >>> uint32_t pixel_format; >>> struct simplefb_format *format = NULL; >>> int i; >>> + u32 reg; >>> >>> pixel_format = crtc->primary->state->fb->pixel_format; >>> >>> @@ -76,7 +77,11 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc) >>> >>> /* HDLCD uses 'bytes per pixel', zero means 1 byte */ >>> btpp = (format->bits_per_pixel + 7) / 8; >>> - hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, (btpp - 1) << 3); >>> + reg = (btpp - 1) << 3; >>> +#ifdef __BIG_ENDIAN >>> + reg |= HDLCD_PIXEL_FMT_BIG_ENDIAN; >>> +#endif >>> + hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, reg); >>> >>> /* >>> * The format of the HDLCD_REG_<color>_SELECT register is: >>> -- >>> 2.10.2.dirty >>> >>> _______________________________________________ >>> dri-devel mailing list >>> dri-devel@xxxxxxxxxxxxxxxxxxxxx >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
