Re: [PATCH 2/3] drm/tiny: ili9486: Do not assume 8-bit only SPI controllers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 17/11/2022 15:59, Mark Brown wrote:

So this is an issue in the MIPI DBI code where the interpretation of the buffer passed in depends on both the a caller parameter and the capabilities of the underlying SPI controller, meaning that a driver can suddenly become buggy when used with a new controller?

The MIPI DBI code is fine, in fact it is doing the correct thing in the
mipi_dbi_typec3_command() function. The problem is that the ILI9486
driver is hijacking that function installing its own hook that is wrong.

I can't really tell what the bits per word being passed in along
with the buffer is supposed to mean, I'd have expected it to
correspond to the format of the buffer but it seems like perhaps the
buffer is always formatted for 16 bits and the callers are needing to
pass in the capabilities of the controller which is then also checked
by the underlying code? This all seems extremely confusing, I'm not surprised there's bugs.

Correct, the buffer (pixel data) is always formatted for 16 bits and the
bpw is to indicate how this data should be put on the bus (according to
the controller capability).

If the controller is only capable of 8-bit transfers, the 16-bit data
needs to be swapped to account for the big endian bus, while this is not
needed if the controller already supports 16-bit transfers.

The decision to swap the data or not is taken in the MIPI DBI code by
probing the controller capabilities, so if the controller only supports
8-bit the data is swapped, otherwise it is not.

The problem arrives when your controller does support 16-bits, so your
data is not swapped, but you still put the data on the bus with 8-bit
transfers.

At the very least your changelog needs to express clearly what is going on, the description doesn't appear to correspond to what
you're changing.

Gotcha, I'll try to clarify that in the next revision.

Thanks,

--
Carlo Caione



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux