Re: [PATCH v8 6/7] drm: atmel-hlcdc: add support for DSI output formats

[claudiu beznea <claudiu.beznea@tuxon.dev>]

On 21.02.2024 07:35, Manikandan Muralidharan wrote:
> Add support for the following DPI mode if the encoder type
> is DSI as per the XLCDC IP datasheet:
> - 16BPPCFG1
> - 16BPPCFG2
> - 16BPPCFG3
> - 18BPPCFG1
> - 18BPPCFG2
> - 24BPP
> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> [durai.manickamkr@microchip.com: update output format using is_xlcdc flag]
> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    | 74 +++++++++++++++++--
>  1 file changed, 68 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index fdd3a6bc0f79..0dbe85686fc2 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -301,11 +301,64 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
>  
>  }
>  
> -#define ATMEL_HLCDC_RGB444_OUTPUT	BIT(0)
> -#define ATMEL_HLCDC_RGB565_OUTPUT	BIT(1)
> -#define ATMEL_HLCDC_RGB666_OUTPUT	BIT(2)
> -#define ATMEL_HLCDC_RGB888_OUTPUT	BIT(3)
> -#define ATMEL_HLCDC_OUTPUT_MODE_MASK	GENMASK(3, 0)
> +#define ATMEL_HLCDC_RGB444_OUTPUT		BIT(0)
> +#define ATMEL_HLCDC_RGB565_OUTPUT		BIT(1)
> +#define ATMEL_HLCDC_RGB666_OUTPUT		BIT(2)
> +#define ATMEL_HLCDC_RGB888_OUTPUT		BIT(3)
> +#define ATMEL_HLCDC_DPI_RGB565C1_OUTPUT		BIT(4)
> +#define ATMEL_HLCDC_DPI_RGB565C2_OUTPUT		BIT(5)
> +#define ATMEL_HLCDC_DPI_RGB565C3_OUTPUT		BIT(6)
> +#define ATMEL_HLCDC_DPI_RGB666C1_OUTPUT		BIT(7)
> +#define ATMEL_HLCDC_DPI_RGB666C2_OUTPUT		BIT(8)
> +#define ATMEL_HLCDC_DPI_RGB888_OUTPUT		BIT(9)
> +#define ATMEL_HLCDC_OUTPUT_MODE_MASK		GENMASK(3, 0)
> +#define ATMEL_XLCDC_OUTPUT_MODE_MASK		GENMASK(9, 0)
> +
> +static int atmel_hlcdc_connector_output_dsi(struct drm_encoder *encoder,
> +					    struct drm_display_info *info)
> +{
> +	int j;
> +	unsigned int supported_fmts = 0;
> +
> +	switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
> +	case 0:
> +		break;
> +	case MEDIA_BUS_FMT_RGB565_1X16:
> +		return ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;
> +	case MEDIA_BUS_FMT_RGB666_1X18:
> +		return ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;
> +	case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
> +		return ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;
> +	case MEDIA_BUS_FMT_RGB888_1X24:
> +		return ATMEL_HLCDC_DPI_RGB888_OUTPUT;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	for (j = 0; j < info->num_bus_formats; j++) {
> +		switch (info->bus_formats[j]) {
> +		case MEDIA_BUS_FMT_RGB565_1X16:
> +			supported_fmts |=
> +				ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;

This can fit on the above line

> +			break;
> +		case MEDIA_BUS_FMT_RGB666_1X18:
> +			supported_fmts |=
> +				ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;

Ditto

> +			break;
> +		case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
> +			supported_fmts |=
> +				ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;

Ditto

> +			break;
> +		case MEDIA_BUS_FMT_RGB888_1X24:
> +			supported_fmts |=
> +				ATMEL_HLCDC_DPI_RGB888_OUTPUT;

Ditto

> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +	return supported_fmts;
> +}
>  
>  static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
>  {
> @@ -318,6 +371,13 @@ static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
>  	encoder = state->best_encoder;
>  	if (!encoder)
>  		encoder = connector->encoder;
> +	/*
> +	 * atmel-hlcdc to support DSI formats with DSI video pipeline
> +	 * when DRM_MODE_ENCODER_DSI type is set by
> +	 * connector driver component.
> +	 */
> +	if (encoder->encoder_type == DRM_MODE_ENCODER_DSI)
> +		return atmel_hlcdc_connector_output_dsi(encoder, info);
>  
>  	switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
>  	case 0:
> @@ -358,7 +418,7 @@ static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
>  
>  static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
>  {
> -	unsigned int output_fmts = ATMEL_HLCDC_OUTPUT_MODE_MASK;
> +	unsigned int output_fmts;
>  	struct atmel_hlcdc_crtc_state *hstate;
>  	struct drm_connector_state *cstate;
>  	struct drm_connector *connector;
> @@ -366,6 +426,8 @@ static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
>  	int i;
>  
>  	crtc = drm_crtc_to_atmel_hlcdc_crtc(state->crtc);
> +	output_fmts = crtc->dc->desc->is_xlcdc ? ATMEL_XLCDC_OUTPUT_MODE_MASK :
> +		      ATMEL_HLCDC_OUTPUT_MODE_MASK;
>  
>  	for_each_new_connector_in_state(state->state, connector, cstate, i) {
>  		unsigned int supported_fmts = 0;