Re: [RESEND PATCH v5 3/3] drm/atmel-hlcdc: support bus-width (12/16/18/24) in endpoint nodes
On 2018-08-03 10:17, jacopo mondi wrote:
> Hi Peter,
>
> On Fri, Aug 03, 2018 at 09:23:08AM +0200, Peter Rosin wrote:
>> This beats the heuristic that the connector is involved in what format
>> should be output for cases where this fails.
>>
>> E.g. if there is a bridge that changes format between the encoder and the
>> connector, or if some of the RGB pins between the lcd controller and the
>> encoder are not routed on the PCB.
>>
>> This is critical for the devices that have the "conflicting output
>> formats" issue (SAM9N12, SAM9X5, SAMA5D3), since the most significant
>> RGB bits move around depending on the selected output mode. For
>> devices that do not have the "conflicting output formats" issue
>> (SAMA5D2, SAMA5D4), this is completely irrelevant.
>>
>> Acked-by: Boris Brezillon
>> Signed-off-by: Peter Rosin
>> ---
>> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 70
>> +---
>> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 1 +
>> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 67
>> ---
>> 3 files changed, 111 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> index d73281095fac..c38a479ada98 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> @@ -226,6 +226,55 @@ static void atmel_hlcdc_crtc_atomic_enable(struct
>> drm_crtc *c,
>> #define ATMEL_HLCDC_RGB888_OUTPUT BIT(3)
>> #define ATMEL_HLCDC_OUTPUT_MODE_MASKGENMASK(3, 0)
>>
>> +static int atmel_hlcdc_connector_output_mode(struct drm_connector_state
>> *state)
>> +{
>> +struct drm_connector *connector = state->connector;
>> +struct drm_display_info *info = >display_info;
>> +struct drm_encoder *encoder;
>> +unsigned int supported_fmts = 0;
>> +int j;
>> +
>> +encoder = state->best_encoder;
>> +if (!encoder)
>> +encoder = connector->encoder;
>> +
>> +switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
>> +case 0:
>> +break;
>> +case MEDIA_BUS_FMT_RGB444_1X12:
>> +return ATMEL_HLCDC_RGB444_OUTPUT;
>> +case MEDIA_BUS_FMT_RGB565_1X16:
>> +return ATMEL_HLCDC_RGB565_OUTPUT;
>> +case MEDIA_BUS_FMT_RGB666_1X18:
>> +return ATMEL_HLCDC_RGB666_OUTPUT;
>> +case MEDIA_BUS_FMT_RGB888_1X24:
>> +return ATMEL_HLCDC_RGB888_OUTPUT;
>> +default:
>> +return -EINVAL;
>> +}
>> +
>> +for (j = 0; j < info->num_bus_formats; j++) {
>> +switch (info->bus_formats[j]) {
>> +case MEDIA_BUS_FMT_RGB444_1X12:
>> +supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
>> +break;
>> +case MEDIA_BUS_FMT_RGB565_1X16:
>> +supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
>> +break;
>> +case MEDIA_BUS_FMT_RGB666_1X18:
>> +supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
>> +break;
>> +case MEDIA_BUS_FMT_RGB888_1X24:
>> +supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
>> +break;
>> +default:
>> +break;
>> +}
>> +}
>> +
>> +return supported_fmts;
>> +}
>> +
>> static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
>> {
>> unsigned int output_fmts = ATMEL_HLCDC_OUTPUT_MODE_MASK;
>> @@ -238,31 +287,12 @@ static int atmel_hlcdc_crtc_select_output_mode(struct
>> drm_crtc_state *state)
>> crtc = drm_crtc_to_atmel_hlcdc_crtc(state->crtc);
>>
>> for_each_new_connector_in_state(state->state, connector, cstate, i) {
>> -struct drm_display_info *info = >display_info;
>> unsigned int supported_fmts = 0;
>> -int j;
>>
>> if (!cstate->crtc)
>> continue;
>>
>> -for (j = 0; j < info->num_bus_formats; j++) {
>> -switch (info->bus_formats[j]) {
>> -case MEDIA_BUS_FMT_RGB444_1X12:
>> -supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
>> -break;
>> -case MEDIA_BUS_FMT_RGB565_1X16:
>> -supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
>> -break;
>> -case MEDIA_BUS_FMT_RGB666_1X18:
>> -supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
>> -break;
>> -case MEDIA_BUS_FMT_RGB888_1X24:
>> -supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
>> -break;
>> -default:
>> -break;
>> -}
>> -}
>> +supported_fmts = atmel_hlcdc_connector_output_mode(cstate);
>>
>>
Re: [RESEND PATCH v5 3/3] drm/atmel-hlcdc: support bus-width (12/16/18/24) in endpoint nodes
Hi Peter,
On Fri, Aug 03, 2018 at 09:23:08AM +0200, Peter Rosin wrote:
> This beats the heuristic that the connector is involved in what format
> should be output for cases where this fails.
>
> E.g. if there is a bridge that changes format between the encoder and the
> connector, or if some of the RGB pins between the lcd controller and the
> encoder are not routed on the PCB.
>
> This is critical for the devices that have the "conflicting output
> formats" issue (SAM9N12, SAM9X5, SAMA5D3), since the most significant
> RGB bits move around depending on the selected output mode. For
> devices that do not have the "conflicting output formats" issue
> (SAMA5D2, SAMA5D4), this is completely irrelevant.
>
> Acked-by: Boris Brezillon
> Signed-off-by: Peter Rosin
> ---
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 70
> +---
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 1 +
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 67 ---
> 3 files changed, 111 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index d73281095fac..c38a479ada98 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -226,6 +226,55 @@ static void atmel_hlcdc_crtc_atomic_enable(struct
> drm_crtc *c,
> #define ATMEL_HLCDC_RGB888_OUTPUTBIT(3)
> #define ATMEL_HLCDC_OUTPUT_MODE_MASK GENMASK(3, 0)
>
> +static int atmel_hlcdc_connector_output_mode(struct drm_connector_state
> *state)
> +{
> + struct drm_connector *connector = state->connector;
> + struct drm_display_info *info = >display_info;
> + struct drm_encoder *encoder;
> + unsigned int supported_fmts = 0;
> + int j;
> +
> + encoder = state->best_encoder;
> + if (!encoder)
> + encoder = connector->encoder;
> +
> + switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
> + case 0:
> + break;
> + case MEDIA_BUS_FMT_RGB444_1X12:
> + return ATMEL_HLCDC_RGB444_OUTPUT;
> + case MEDIA_BUS_FMT_RGB565_1X16:
> + return ATMEL_HLCDC_RGB565_OUTPUT;
> + case MEDIA_BUS_FMT_RGB666_1X18:
> + return ATMEL_HLCDC_RGB666_OUTPUT;
> + case MEDIA_BUS_FMT_RGB888_1X24:
> + return ATMEL_HLCDC_RGB888_OUTPUT;
> + default:
> + return -EINVAL;
> + }
> +
> + for (j = 0; j < info->num_bus_formats; j++) {
> + switch (info->bus_formats[j]) {
> + case MEDIA_BUS_FMT_RGB444_1X12:
> + supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
> + break;
> + case MEDIA_BUS_FMT_RGB565_1X16:
> + supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
> + break;
> + case MEDIA_BUS_FMT_RGB666_1X18:
> + supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
> + break;
> + case MEDIA_BUS_FMT_RGB888_1X24:
> + supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
> + break;
> + default:
> + break;
> + }
> + }
> +
> + return supported_fmts;
> +}
> +
> static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
> {
> unsigned int output_fmts = ATMEL_HLCDC_OUTPUT_MODE_MASK;
> @@ -238,31 +287,12 @@ static int atmel_hlcdc_crtc_select_output_mode(struct
> drm_crtc_state *state)
> crtc = drm_crtc_to_atmel_hlcdc_crtc(state->crtc);
>
> for_each_new_connector_in_state(state->state, connector, cstate, i) {
> - struct drm_display_info *info = >display_info;
> unsigned int supported_fmts = 0;
> - int j;
>
> if (!cstate->crtc)
> continue;
>
> - for (j = 0; j < info->num_bus_formats; j++) {
> - switch (info->bus_formats[j]) {
> - case MEDIA_BUS_FMT_RGB444_1X12:
> - supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
> - break;
> - case MEDIA_BUS_FMT_RGB565_1X16:
> - supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
> - break;
> - case MEDIA_BUS_FMT_RGB666_1X18:
> - supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
> - break;
> - case MEDIA_BUS_FMT_RGB888_1X24:
> - supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
> - break;
> - default:
> - break;
> - }
> - }
> + supported_fmts = atmel_hlcdc_connector_output_mode(cstate);
>
> if (crtc->dc->desc->conflicting_output_formats)
> output_fmts &= supported_fmts;
>
