[PATCH] drm/exynos: set the active aspect ratio as per mode

[Shirish S]

Hi Tomasz,

On Wed, Mar 12, 2014 at 8:19 AM, Tomasz Figa <t.figa at samsung.com> wrote:
> Hi Shirish,
>
>
> On 10.03.2014 04:44, Shirish S wrote:
>>
>> now that the drm_display_mode also provides aspect
>> ratio for all resolutions, this patch adds its usage
>> to set the active aspect ratio of AVI info frame
>> packets as per CEA-861-D standard's Table 9.
>>
>> This is also needed to abide by the 7-27
>> compliance test of HDMI.
>>
>> Signed-off-by: Shirish S <s.shirish at samsung.com>
>> ---
>>   drivers/gpu/drm/exynos/exynos_hdmi.c |   33
>> +++++++++++++++++++++++++++------
>>   1 file changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index c021ddc..8aca52a 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -53,8 +53,6 @@
>>   /* AVI header and aspect ratio */
>>   #define HDMI_AVI_VERSION              0x02
>>   #define HDMI_AVI_LENGTH               0x0D
>> -#define AVI_PIC_ASPECT_RATIO_16_9      (2 << 4)
>> -#define AVI_SAME_AS_PIC_ASPECT_RATIO   8
>>
>>   /* AUI header info */
>>   #define HDMI_AUI_VERSION      0x01
>> @@ -65,6 +63,12 @@ enum hdmi_type {
>>         HDMI_TYPE14,
>>   };
>>
>> +enum active_aspect_ratio {
>> +       AVI_SAME_AS_PIC_ASPECT_RATIO = 8,
>> +       AVI_4_3_Center_RATIO,
>> +       AVI_16_9_Center_RATIO,
>> +};
>> +
>
>
> CodingStyle: Please define these using preprocessor macros, since they are
> bitfield values. Also coding style implies using uppercase for constants.
>
>
Agreed, updated in the next path set.
>>   struct hdmi_resources {
>>         struct clk                      *hdmi;
>>         struct clk                      *sclk_hdmi;
>> @@ -162,6 +166,7 @@ struct hdmi_v14_conf {
>>   struct hdmi_conf_regs {
>>         int pixel_clock;
>>         int cea_video_id;
>> +       enum hdmi_picture_aspect aspect_ratio;
>>         union {
>>                 struct hdmi_v13_conf v13_conf;
>>                 struct hdmi_v14_conf v14_conf;
>> @@ -668,7 +673,6 @@ static void hdmi_reg_infoframe(struct hdmi_context
>> *hdata,
>>   {
>>         u32 hdr_sum;
>>         u8 chksum;
>> -       u32 aspect_ratio;
>>         u32 mod;
>>         u32 vic;
>>
>> @@ -697,10 +701,25 @@ static void hdmi_reg_infoframe(struct hdmi_context
>> *hdata,
>>                         AVI_ACTIVE_FORMAT_VALID |
>>                         AVI_UNDERSCANNED_DISPLAY_VALID);
>>
>> -               aspect_ratio = AVI_PIC_ASPECT_RATIO_16_9;
>> -
>> -               hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(2), aspect_ratio |
>> +               /*
>> +                * Set the aspect ratio as per the mode, mentioned in
>> +                * Table 9 AVI InfoFrame Data Byte 2 of CEA-861-D Standard
>> +                */
>> +               switch (hdata->mode_conf.aspect_ratio) {
>> +               case HDMI_PICTURE_ASPECT_4_3:
>> +                       hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(2),
>> aspect_ratio |
>> +                               AVI_4_3_Center_RATIO);
>
>
> CodingStyle: Please keep wrapped function calls aligned using tabs at least
> to the opening parenthesis.
>
>
>> +                       break;
>> +               case HDMI_PICTURE_ASPECT_16_9:
>> +                       hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(2),
>> aspect_ratio |
>> +                               AVI_16_9_Center_RATIO);
>
>
> Ditto.
>
Agreed, updated in next patch set.
>
>> +                       break;
>> +               case HDMI_PICTURE_ASPECT_NONE:
>> +               default:
>> +                       hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(2),
>> aspect_ratio |
>>                                 AVI_SAME_AS_PIC_ASPECT_RATIO);
>
>
> Ditto.
>
> Best regards,
> Tomasz
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Thanks & Regards,
Shirish S