Re: [PATCH v2 2/4] drm/ssd130x: Add RGB565 support to SSD133X family

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Mon, Jun 22, 2026 at 06:25:04PM +0300, Amit Barzilai wrote:
> SSD133X screens were getting 8bpp (RGB332) instead of the 16bpp
> (RGB565) that they support. This change adds a boolean to the
> deviceinfo struct selecting whether the variant is driven at
> DRM_FORMAT_RGB565.
> 
> Changed SSD133X to now utilize 65k color (RGB565).

...

> -	 * Each Segment has a 8-bit pixel and each Common output has a
> -	 * row of pixels. When using the (default) horizontal address
> -	 * increment mode, each byte of data sent to the controller has
> -	 * a Segment (e.g: SEG0).
> +	 * Each Segment holds one pixel and each Common output has a row
> +	 * of pixels. A pixel is 8 bits (one byte) in the 256 color
> +	 * (RGB332) format or 16 bits (two bytes) in the 65k color
> +	 * (RGB565) format. When using the (default) horizontal address
> +	 * increment mode, the pixel data is sent Segment by Segment
> +	 * (e.g: SEG0 first).
>  	 *
>  	 * When using the 256 color depth format, each pixel contains 3
>  	 * sub-pixels for color A, B and C. These have 3 bit, 3 bit and
>  	 * 2 bits respectively.

Something wrong with the plural. There is a difference between "3-bit" and
"3 bits", but "3 bit" is odd.

> +	 *
> +	 * When using the 65k color depth format, each pixel contains 3
> +	 * sub-pixels for color A, B and C. These have 5 bit, 6 bit and
> +	 * 5 bits respectively.

Same mistake is repeated here.

...

> +	/*
> +	 * Per-variant output format selector for the SSD133X data path. The
> +	 * hardware can drive the panel in RGB332 (1 byte/pixel) or RGB565
> +	 * (2 bytes/pixel); this is a policy choice per variant, not a

In other comments it was spelled fully, be consistent "1 byte per pixel",
"2 bytes per pixel".

> +	 * capability probe. When set, the variant is driven at RGB565.
> +	 */

-- 
With Best Regards,
Andy Shevchenko