Re: [PATCH v2 1/5] drm/dsi: clean up DSI data type definitions

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3797 bytes --]

On Mon, Oct 28, 2019 at 05:00:43PM +0200, Jani Nikula wrote:
> Rename picture parameter set (it's a long packet, not a long write) and
> compression mode (it's not a DCS command) enumerations according to the
> DSI specification. Order the types according to the spec. Use tabs
> instead of spaces for indentation. Use all lower case for hex.
> 
> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com>
> Reviewed-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_mipi_dsi.c |  4 ++--
>  include/video/mipi_display.h   | 10 +++++-----
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index bd2498bbd74a..f237d80828c3 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -373,6 +373,7 @@ bool mipi_dsi_packet_format_is_short(u8 type)
>  	case MIPI_DSI_V_SYNC_END:
>  	case MIPI_DSI_H_SYNC_START:
>  	case MIPI_DSI_H_SYNC_END:
> +	case MIPI_DSI_COMPRESSION_MODE:
>  	case MIPI_DSI_END_OF_TRANSMISSION:
>  	case MIPI_DSI_COLOR_MODE_OFF:
>  	case MIPI_DSI_COLOR_MODE_ON:
> @@ -387,7 +388,6 @@ bool mipi_dsi_packet_format_is_short(u8 type)
>  	case MIPI_DSI_DCS_SHORT_WRITE:
>  	case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
>  	case MIPI_DSI_DCS_READ:
> -	case MIPI_DSI_DCS_COMPRESSION_MODE:
>  	case MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE:
>  		return true;
>  	}
> @@ -406,11 +406,11 @@ EXPORT_SYMBOL(mipi_dsi_packet_format_is_short);
>  bool mipi_dsi_packet_format_is_long(u8 type)
>  {
>  	switch (type) {
> -	case MIPI_DSI_PPS_LONG_WRITE:
>  	case MIPI_DSI_NULL_PACKET:
>  	case MIPI_DSI_BLANKING_PACKET:
>  	case MIPI_DSI_GENERIC_LONG_WRITE:
>  	case MIPI_DSI_DCS_LONG_WRITE:
> +	case MIPI_DSI_PICTURE_PARAMETER_SET:
>  	case MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20:
>  	case MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24:
>  	case MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16:
> diff --git a/include/video/mipi_display.h b/include/video/mipi_display.h
> index cba57a678daf..79fd71cf4934 100644
> --- a/include/video/mipi_display.h
> +++ b/include/video/mipi_display.h
> @@ -17,6 +17,9 @@ enum {
>  	MIPI_DSI_H_SYNC_START				= 0x21,
>  	MIPI_DSI_H_SYNC_END				= 0x31,
>  
> +	MIPI_DSI_COMPRESSION_MODE			= 0x07,
> +	MIPI_DSI_END_OF_TRANSMISSION			= 0x08,
> +
>  	MIPI_DSI_COLOR_MODE_OFF				= 0x02,
>  	MIPI_DSI_COLOR_MODE_ON				= 0x12,
>  	MIPI_DSI_SHUTDOWN_PERIPHERAL			= 0x22,
> @@ -35,18 +38,15 @@ enum {
>  
>  	MIPI_DSI_DCS_READ				= 0x06,
>  
> -	MIPI_DSI_DCS_COMPRESSION_MODE                   = 0x07,
> -	MIPI_DSI_PPS_LONG_WRITE                         = 0x0A,
> -
>  	MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE		= 0x37,
>  
> -	MIPI_DSI_END_OF_TRANSMISSION			= 0x08,
> -
>  	MIPI_DSI_NULL_PACKET				= 0x09,
>  	MIPI_DSI_BLANKING_PACKET			= 0x19,
>  	MIPI_DSI_GENERIC_LONG_WRITE			= 0x29,
>  	MIPI_DSI_DCS_LONG_WRITE				= 0x39,
>  
> +	MIPI_DSI_PICTURE_PARAMETER_SET			= 0x0a,
> +
>  	MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20	= 0x0c,
>  	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24		= 0x1c,
>  	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16		= 0x2c,

Looks good to me. I haven't specifically checked that the order matches
that in the specification, but given that it's not really ordered in any
sane way in the first place (or perhaps I'm too stupid to see the logic)
I don't really mind about the order.

Took me a while to find the specification. You might want to mention in
the commit message that some of these enumerations are only available in
the DSI 2 specification because I was looking at 1.2 first. Anyway, the
enumerations and names match what's in the spec, so:

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx