From: Ricardo Neri <ricardo.neri@ti.com>
To: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: mythripk@ti.com, s-chereau@ti.com, x0055901@ti.com,
vaibhav.bedia@ti.com, s-guiriec@ti.com, lrg@ti.com,
peter.ujfalusi@ti.com, agraf@suse.de, research@ottomaneng.com,
linux-omap@vger.kernel.org
Subject: Re: [PATCH 02/10] OMAPDSS: HDMI: OMAP4: Remove CEA-861 audio infoframe and IEC-60958 enums
Date: Tue, 24 Apr 2012 22:37:41 -0500 [thread overview]
Message-ID: <4F977185.1060107@ti.com> (raw)
In-Reply-To: <1335186769.1535.30.camel@lappy>
On 04/23/2012 08:12 AM, Tomi Valkeinen wrote:
> On Wed, 2012-03-28 at 16:38 -0600, Ricardo Neri wrote:
>> In order to avoid duplication of definitions. Use the definitions provided
>> by asoundef.h. Furthermore, as CEA-861 and IEC-60958 are used by both
>> DisplayPort and HDMI, this helps to make the code more generic.
>
> The first two sentences should probably be just one sentence.
Actually, I just rephrased the whole description :). I will submit it in
v2 of the patch series.
>
>> Signed-off-by: Ricardo Neri<ricardo.neri@ti.com>
>> ---
>> drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 15 +++---
>> drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h | 80 ++---------------------------
>> 2 files changed, 12 insertions(+), 83 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
>> index 4740e64..4ab3b19 100644
>> --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
>> +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
>> @@ -29,7 +29,6 @@
>> #include<linux/string.h>
>> #include<linux/seq_file.h>
>> #include<linux/gpio.h>
>> -
>
> Unrelated change.
I will remove it.
>
>> #include "ti_hdmi_4xxx_ip.h"
>> #include "dss.h"
>>
>> @@ -1156,7 +1155,7 @@ void hdmi_core_audio_config(struct hdmi_ip_data *ip_data,
>> }
>>
>> void hdmi_core_audio_infoframe_config(struct hdmi_ip_data *ip_data,
>> - struct hdmi_core_infoframe_audio *info_aud)
>> + struct snd_cea_861_aud_if *info_aud)
>> {
>> u8 val;
>> u8 sum = 0, checksum = 0;
>> @@ -1172,22 +1171,24 @@ void hdmi_core_audio_infoframe_config(struct hdmi_ip_data *ip_data,
>> hdmi_write_reg(av_base, HDMI_CORE_AV_AUDIO_LEN, 0x0a);
>> sum += 0x84 + 0x001 + 0x00a;
>>
>> - val = (info_aud->db1_coding_type<< 4)
>> - | (info_aud->db1_channel_count - 1);
>> + val = (info_aud->coding_type<< CEA861_AUDIO_INFOFRAME_DB1CT_SHIFT)
>> + | (info_aud->channel_count - 1);
>> hdmi_write_reg(av_base, HDMI_CORE_AV_AUD_DBYTE(0), val);
>> sum += val;
>>
>> - val = (info_aud->db2_sample_freq<< 2) | info_aud->db2_sample_size;
>> + val = (info_aud->sample_freq<< CEA861_AUDIO_INFOFRAME_DB2SF_SHIFT)
>> + | (info_aud->sample_size);
>> hdmi_write_reg(av_base, HDMI_CORE_AV_AUD_DBYTE(1), val);
>> sum += val;
>>
>> hdmi_write_reg(av_base, HDMI_CORE_AV_AUD_DBYTE(2), 0x00);
>>
>> - val = info_aud->db4_channel_alloc;
>> + val = info_aud->channel_alloc;
>> hdmi_write_reg(av_base, HDMI_CORE_AV_AUD_DBYTE(3), val);
>> sum += val;
>>
>> - val = (info_aud->db5_downmix_inh<< 7) | (info_aud->db5_lsv<< 3);
>> + val = (info_aud->st_downmix<< CEA861_AUDIO_INFOFRAME_DB5_DM_INH_SHIFT)
>> + | (info_aud->lsv<< CEA861_AUDIO_INFOFRAME_DB5_LSV_SHIFT);
>> hdmi_write_reg(av_base, HDMI_CORE_AV_AUD_DBYTE(4), val);
>> sum += val;
>>
>> diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h
>> index a442998..d6b49b6 100644
>> --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h
>> +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h
>> @@ -28,6 +28,8 @@
>> defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
>> #include<sound/soc.h>
>> #include<sound/pcm_params.h>
>> +#include<sound/asound.h>
>> +#include<sound/asoundef.h>
>
> You don't need to include these in the header file.
I will remove it.
>
>> #endif
>>
>> /* HDMI Wrapper */
>> @@ -284,35 +286,6 @@ enum hdmi_core_infoframe {
>> HDMI_INFOFRAME_AVI_DB5PR_8 = 7,
>> HDMI_INFOFRAME_AVI_DB5PR_9 = 8,
>> HDMI_INFOFRAME_AVI_DB5PR_10 = 9,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_FROM_STREAM = 0,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_IEC60958 = 1,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_AC3 = 2,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_MPEG1 = 3,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_MP3 = 4,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_MPEG2_MULTICH = 5,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_AAC = 6,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_DTS = 7,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_ATRAC = 8,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_ONEBIT = 9,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_DOLBY_DIGITAL_PLUS = 10,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_DTS_HD = 11,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_MAT = 12,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_DST = 13,
>> - HDMI_INFOFRAME_AUDIO_DB1CT_WMA_PRO = 14,
>> - HDMI_INFOFRAME_AUDIO_DB2SF_FROM_STREAM = 0,
>> - HDMI_INFOFRAME_AUDIO_DB2SF_32000 = 1,
>> - HDMI_INFOFRAME_AUDIO_DB2SF_44100 = 2,
>> - HDMI_INFOFRAME_AUDIO_DB2SF_48000 = 3,
>> - HDMI_INFOFRAME_AUDIO_DB2SF_88200 = 4,
>> - HDMI_INFOFRAME_AUDIO_DB2SF_96000 = 5,
>> - HDMI_INFOFRAME_AUDIO_DB2SF_176400 = 6,
>> - HDMI_INFOFRAME_AUDIO_DB2SF_192000 = 7,
>> - HDMI_INFOFRAME_AUDIO_DB2SS_FROM_STREAM = 0,
>> - HDMI_INFOFRAME_AUDIO_DB2SS_16BIT = 1,
>> - HDMI_INFOFRAME_AUDIO_DB2SS_20BIT = 2,
>> - HDMI_INFOFRAME_AUDIO_DB2SS_24BIT = 3,
>> - HDMI_INFOFRAME_AUDIO_DB5_DM_INH_PERMITTED = 0,
>> - HDMI_INFOFRAME_AUDIO_DB5_DM_INH_PROHIBITED = 1
>> };
>>
>> enum hdmi_packing_mode {
>> @@ -322,17 +295,6 @@ enum hdmi_packing_mode {
>> HDMI_PACK_ALREADYPACKED = 7
>> };
>>
>> -enum hdmi_core_audio_sample_freq {
>> - HDMI_AUDIO_FS_32000 = 0x3,
>> - HDMI_AUDIO_FS_44100 = 0x0,
>> - HDMI_AUDIO_FS_48000 = 0x2,
>> - HDMI_AUDIO_FS_88200 = 0x8,
>> - HDMI_AUDIO_FS_96000 = 0xA,
>> - HDMI_AUDIO_FS_176400 = 0xC,
>> - HDMI_AUDIO_FS_192000 = 0xE,
>> - HDMI_AUDIO_FS_NOT_INDICATED = 0x1
>> -};
>> -
>> enum hdmi_core_audio_layout {
>> HDMI_AUDIO_LAYOUT_2CH = 0,
>> HDMI_AUDIO_LAYOUT_8CH = 1
>> @@ -393,31 +355,10 @@ enum hdmi_audio_i2s_config {
>
> Are the defines left in the hdmi_audio_i2s_config something that are IP
> specific? Are they even used? I'm just wondering why many of the defines
> are in sound headers, but some are left here.
Some are specific to the OMAP4 HDMI IP, such as HDMI_AUDIO_I2S_SDx_EN.
Some others refer to generic I2S concepts (such as
I2S_SCK_EDGE_FALLING/RISING) but defines are used to configure registers
and such configuration may be different in other hardware. The defines
that this patch removes are values that are effectively transmitted to
the sink and are clearly defined in the relevant standards. Anyways, I
will look at it further to see if some of them can be removed as well.
Also, the I2S is the same for most of the supported use-cases, if not
all of them. Maybe I can remove the unused ones.
>
>> HDMI_AUDIO_I2S_LSB_SHIFTED_FIRST = 1,
>> HDMI_AUDIO_I2S_MAX_WORD_20BITS = 0,
>> HDMI_AUDIO_I2S_MAX_WORD_24BITS = 1,
>> - HDMI_AUDIO_I2S_CHST_WORD_NOT_SPECIFIED = 0,
>> - HDMI_AUDIO_I2S_CHST_WORD_16_BITS = 1,
>> - HDMI_AUDIO_I2S_CHST_WORD_17_BITS = 6,
>> - HDMI_AUDIO_I2S_CHST_WORD_18_BITS = 2,
>> - HDMI_AUDIO_I2S_CHST_WORD_19_BITS = 4,
>> - HDMI_AUDIO_I2S_CHST_WORD_20_BITS_20MAX = 5,
>> - HDMI_AUDIO_I2S_CHST_WORD_20_BITS_24MAX = 1,
>> - HDMI_AUDIO_I2S_CHST_WORD_21_BITS = 6,
>> - HDMI_AUDIO_I2S_CHST_WORD_22_BITS = 2,
>> - HDMI_AUDIO_I2S_CHST_WORD_23_BITS = 4,
>> - HDMI_AUDIO_I2S_CHST_WORD_24_BITS = 5,
>> HDMI_AUDIO_I2S_SCK_EDGE_FALLING = 0,
>> HDMI_AUDIO_I2S_SCK_EDGE_RISING = 1,
>> HDMI_AUDIO_I2S_VBIT_FOR_PCM = 0,
>> HDMI_AUDIO_I2S_VBIT_FOR_COMPRESSED = 1,
>> - HDMI_AUDIO_I2S_INPUT_LENGTH_NA = 0,
>> - HDMI_AUDIO_I2S_INPUT_LENGTH_16 = 2,
>> - HDMI_AUDIO_I2S_INPUT_LENGTH_17 = 12,
>> - HDMI_AUDIO_I2S_INPUT_LENGTH_18 = 4,
>> - HDMI_AUDIO_I2S_INPUT_LENGTH_19 = 8,
>> - HDMI_AUDIO_I2S_INPUT_LENGTH_20 = 10,
>> - HDMI_AUDIO_I2S_INPUT_LENGTH_21 = 13,
>> - HDMI_AUDIO_I2S_INPUT_LENGTH_22 = 5,
>> - HDMI_AUDIO_I2S_INPUT_LENGTH_23 = 9,
>> - HDMI_AUDIO_I2S_INPUT_LENGTH_24 = 11,
>> HDMI_AUDIO_I2S_FIRST_BIT_SHIFT = 0,
>> HDMI_AUDIO_I2S_FIRST_BIT_NO_SHIFT = 1,
>> HDMI_AUDIO_I2S_SD0_EN = 1,
>> @@ -486,19 +427,6 @@ struct hdmi_core_infoframe_avi {
>> /* Pixel number start of right bar */
>> u16 db12_13_pixel_sofright;
>> };
>> -/*
>> - * Refer to section 8.2 in HDMI 1.3 specification for
>> - * details about infoframe databytes
>> - */
>> -struct hdmi_core_infoframe_audio {
>> - u8 db1_coding_type;
>> - u8 db1_channel_count;
>> - u8 db2_sample_freq;
>> - u8 db2_sample_size;
>> - u8 db4_channel_alloc;
>> - bool db5_downmix_inh;
>> - u8 db5_lsv; /* Level shift values for downmix */
>> -};
>>
>> struct hdmi_core_packet_enable_repeat {
>> u32 audio_pkt;
>> @@ -559,7 +487,7 @@ struct hdmi_core_audio_i2s_config {
>>
>> struct hdmi_core_audio_config {
>> struct hdmi_core_audio_i2s_config i2s_cfg;
>> - enum hdmi_core_audio_sample_freq freq_sample;
>> + u32 freq_sample;
>
> Try to keep consistent code formatting. If the original code indents the
> field names, indent your field name also.
I will fix the indentation.
BR,
Ricardo
>
> Tomi
next prev parent reply other threads:[~2012-04-25 3:37 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-28 22:38 [PATCH 00/10] OMAPDSS: HDMI: Prepare for OMAP5 and DSS dev driver audio support Ricardo Neri
2012-03-28 22:38 ` [PATCH 01/10] OMAPDSS: HDMI: Remove ASoC codec Ricardo Neri
2012-04-23 13:17 ` Tomi Valkeinen
2012-04-25 2:27 ` Ricardo Neri
2012-03-28 22:38 ` [PATCH 02/10] OMAPDSS: HDMI: OMAP4: Remove CEA-861 audio infoframe and IEC-60958 enums Ricardo Neri
2012-04-23 13:12 ` Tomi Valkeinen
2012-04-25 3:37 ` Ricardo Neri [this message]
2012-04-27 1:32 ` Ricardo Neri
2012-04-27 6:31 ` Tomi Valkeinen
2012-03-28 22:38 ` [PATCH 03/10] OMAPDSS: HDMI: OMAP4: Correcty typo in I2S definitions Ricardo Neri
2012-04-23 12:42 ` Tomi Valkeinen
2012-04-25 3:39 ` Ricardo Neri
2012-03-28 22:38 ` [PATCH 04/10] OMAPDSS: HDMI: OMAP4: Decouple wrapper enable and audio start Ricardo Neri
2012-03-28 22:38 ` [PATCH 05/10] OMAPDSS: HDMI: Decouple HDMI audio from ASoC Ricardo Neri
2012-04-23 13:25 ` Tomi Valkeinen
2012-04-25 3:44 ` Ricardo Neri
2012-03-28 22:38 ` [PATCH 06/10] OMAPDSS: HDMI: OMAP4: Expand configuration for IEC-60958 audio Ricardo Neri
2012-03-28 22:38 ` [PATCH 07/10] OMAPDSS: HDMI: Relocate N/CTS calculation Ricardo Neri
2012-03-28 22:38 ` [PATCH 08/10] OMAPDSS: HDMI: Add support for more audio sample rates in " Ricardo Neri
2012-03-28 22:38 ` [PATCH 09/10] OMAPDSS: HDMI: OMAP4: Add an audio configuration function Ricardo Neri
2012-03-28 22:38 ` [PATCH 10/10] OMAPDSS: HDMI: Implement DSS driver interface for audio Ricardo Neri
2012-04-23 13:01 ` Tomi Valkeinen
2012-04-25 4:48 ` Ricardo Neri
2012-04-25 6:19 ` Tomi Valkeinen
2012-04-25 23:01 ` Ricardo Neri
2012-04-26 7:31 ` Tomi Valkeinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F977185.1060107@ti.com \
--to=ricardo.neri@ti.com \
--cc=agraf@suse.de \
--cc=linux-omap@vger.kernel.org \
--cc=lrg@ti.com \
--cc=mythripk@ti.com \
--cc=peter.ujfalusi@ti.com \
--cc=research@ottomaneng.com \
--cc=s-chereau@ti.com \
--cc=s-guiriec@ti.com \
--cc=tomi.valkeinen@ti.com \
--cc=vaibhav.bedia@ti.com \
--cc=x0055901@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).