diff mbox

[3/8] OMAP4: DSS: HDMI: Use specific HDMI timings structure

Message ID 1308298637-29753-4-git-send-email-mythripk@ti.com (mailing list archive)
State Rejected
Delegated to: Tomi Valkeinen
Headers show

Commit Message

K, Mythri P June 17, 2011, 8:17 a.m. UTC
Define new HDMI timings structure to replace the OMAP DSS timing strucutre in hdmi.c
to have the HDMI include defintion out of DSS.

Signed-off-by: Mythri P K <mythripk@ti.com>
---
 drivers/video/omap2/dss/hdmi.c |   23 ++++++++++++++++++++---
 drivers/video/omap2/dss/hdmi.h |   15 ++++++++++++++-
 2 files changed, 34 insertions(+), 4 deletions(-)

Comments

Sanjeev Premi June 20, 2011, 12:46 p.m. UTC | #1
> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org 
> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of K, Mythri P
> Sent: Friday, June 17, 2011 1:47 PM
> To: linux-omap@vger.kernel.org; Valkeinen, Tomi
> Cc: K, Mythri P
> Subject: [PATCH 3/8] OMAP4: DSS: HDMI: Use specific HDMI 
> timings structure
> 
> Define new HDMI timings structure to replace the OMAP DSS 
> timing strucutre in hdmi.c

[sp] Fix spelling of structure.

> to have the HDMI include defintion out of DSS.
> 
> Signed-off-by: Mythri P K <mythripk@ti.com>
> ---
>  drivers/video/omap2/dss/hdmi.c |   23 ++++++++++++++++++++---
>  drivers/video/omap2/dss/hdmi.h |   15 ++++++++++++++-
>  2 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/hdmi.c 
> b/drivers/video/omap2/dss/hdmi.c
> index 4ee879a..c24d573 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -593,6 +593,20 @@ static int read_edid(struct hdmi_ip_data 
> *ip_data, u8 *pedid, u16 max_length)
>  	return 0;
>  }
>  
> +static void copy_hdmi_to_dss_timings(struct 
> hdmi_video_timings hdmi_timings,
> +			struct omap_video_timings *timings)

[sp] Assuming that we really need 2 separate structures, suggest
     renaming function to "copy_hdmi_timings_to_dss" or
     "copy_timings_hdmi_to_dss" for better readability.

> +{
> +	timings->x_res = hdmi_timings.x_res;
> +	timings->y_res = hdmi_timings.y_res;
> +	timings->pixel_clock = hdmi_timings.pixel_clock;
> +	timings->hbp = hdmi_timings.hbp;
> +	timings->hfp = hdmi_timings.hfp;
> +	timings->hsw = hdmi_timings.hsw;
> +	timings->vbp = hdmi_timings.vbp;
> +	timings->vfp = hdmi_timings.vfp;
> +	timings->vsw = hdmi_timings.vsw;
> +}
> +
>  static int get_timings_index(void)
>  {
>  	int code;
> @@ -617,7 +631,7 @@ static struct hdmi_cm 
> hdmi_get_code(struct omap_video_timings *timing)
>  {
>  	int i = 0, code = -1, temp_vsync = 0, temp_hsync = 0;
>  	int timing_vsync = 0, timing_hsync = 0;
> -	struct omap_video_timings temp;
> +	struct hdmi_video_timings temp;
>  	struct hdmi_cm cm = {-1};
>  	DSSDBG("hdmi_get_code\n");
>  
> @@ -775,7 +789,8 @@ static void hdmi_read_edid(struct 
> omap_video_timings *dp)
>  
>  	code = get_timings_index();
>  
> -	*dp = cea_vesa_timings[code].timings;
> +	copy_hdmi_to_dss_timings(cea_vesa_timings[code].timings, dp);
> +
>  }
>  
>  static void hdmi_core_init(struct hdmi_core_video_config *video_cfg,
> @@ -1234,7 +1249,9 @@ static int hdmi_power_on(struct 
> omap_dss_device *dssdev)
>  		hdmi_read_edid(p);
>  	}
>  	code = get_timings_index();
> -	dssdev->panel.timings = cea_vesa_timings[code].timings;
> +	copy_hdmi_to_dss_timings(cea_vesa_timings[code].timings,
> +			&dssdev->panel.timings);
> +
>  	update_hdmi_timings(&hdmi.cfg, p, code);
>  
>  	phy = p->pixel_clock;
> diff --git a/drivers/video/omap2/dss/hdmi.h 
> b/drivers/video/omap2/dss/hdmi.h
> index f57ef4a..dcc30b7 100644
> --- a/drivers/video/omap2/dss/hdmi.h
> +++ b/drivers/video/omap2/dss/hdmi.h
> @@ -188,9 +188,22 @@ struct hdmi_reg { u16 idx; };
>  #define REG_GET(base, idx, start, end) \
>  	FLD_GET(hdmi_read_reg(base, idx), start, end)
>  
> +struct hdmi_video_timings {
> +	u16 x_res;
> +	u16 y_res;
> +	/* Unit: KHz */
> +	u32 pixel_clock;

[sp] Could be:
	u32 pixel_clock; /* unit: KHz */

> +	u16 hsw;
> +	u16 hfp;
> +	u16 hbp;
> +	u16 vsw;
> +	u16 vfp;
> +	u16 vbp;
> +};
> +
>  /* HDMI timing structure */
>  struct hdmi_timings {
> -	struct omap_video_timings timings;
> +	struct hdmi_video_timings timings;
>  	int vsync_pol;
>  	int hsync_pol;
>  };
> -- 
> 1.7.5.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
K, Mythri P June 23, 2011, 5:45 a.m. UTC | #2
Hi Sanjeev,

On Mon, Jun 20, 2011 at 6:16 PM, Premi, Sanjeev <premi@ti.com> wrote:
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org
>> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of K, Mythri P
>> Sent: Friday, June 17, 2011 1:47 PM
>> To: linux-omap@vger.kernel.org; Valkeinen, Tomi
>> Cc: K, Mythri P
>> Subject: [PATCH 3/8] OMAP4: DSS: HDMI: Use specific HDMI
>> timings structure
>>
>> Define new HDMI timings structure to replace the OMAP DSS
>> timing strucutre in hdmi.c
>
> [sp] Fix spelling of structure.
>
Thanks will fix it.
>> to have the HDMI include defintion out of DSS.
>>
>> Signed-off-by: Mythri P K <mythripk@ti.com>
>> ---
>>  drivers/video/omap2/dss/hdmi.c |   23 ++++++++++++++++++++---
>>  drivers/video/omap2/dss/hdmi.h |   15 ++++++++++++++-
>>  2 files changed, 34 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/hdmi.c
>> b/drivers/video/omap2/dss/hdmi.c
>> index 4ee879a..c24d573 100644
>> --- a/drivers/video/omap2/dss/hdmi.c
>> +++ b/drivers/video/omap2/dss/hdmi.c
>> @@ -593,6 +593,20 @@ static int read_edid(struct hdmi_ip_data
>> *ip_data, u8 *pedid, u16 max_length)
>>       return 0;
>>  }
>>
>> +static void copy_hdmi_to_dss_timings(struct
>> hdmi_video_timings hdmi_timings,
>> +                     struct omap_video_timings *timings)
>
> [sp] Assuming that we really need 2 separate structures, suggest
>     renaming function to "copy_hdmi_timings_to_dss" or
>     "copy_timings_hdmi_to_dss" for better readability.
>
I shall rename.
>> +{
>> +     timings->x_res = hdmi_timings.x_res;
>> +     timings->y_res = hdmi_timings.y_res;
>> +     timings->pixel_clock = hdmi_timings.pixel_clock;
>> +     timings->hbp = hdmi_timings.hbp;
>> +     timings->hfp = hdmi_timings.hfp;
>> +     timings->hsw = hdmi_timings.hsw;
>> +     timings->vbp = hdmi_timings.vbp;
>> +     timings->vfp = hdmi_timings.vfp;
>> +     timings->vsw = hdmi_timings.vsw;
>> +}
>> +
>>  static int get_timings_index(void)
>>  {
>>       int code;
>> @@ -617,7 +631,7 @@ static struct hdmi_cm
>> hdmi_get_code(struct omap_video_timings *timing)
>>  {
>>       int i = 0, code = -1, temp_vsync = 0, temp_hsync = 0;
>>       int timing_vsync = 0, timing_hsync = 0;
>> -     struct omap_video_timings temp;
>> +     struct hdmi_video_timings temp;
>>       struct hdmi_cm cm = {-1};
>>       DSSDBG("hdmi_get_code\n");
>>
>> @@ -775,7 +789,8 @@ static void hdmi_read_edid(struct
>> omap_video_timings *dp)
>>
>>       code = get_timings_index();
>>
>> -     *dp = cea_vesa_timings[code].timings;
>> +     copy_hdmi_to_dss_timings(cea_vesa_timings[code].timings, dp);
>> +
>>  }
>>
>>  static void hdmi_core_init(struct hdmi_core_video_config *video_cfg,
>> @@ -1234,7 +1249,9 @@ static int hdmi_power_on(struct
>> omap_dss_device *dssdev)
>>               hdmi_read_edid(p);
>>       }
>>       code = get_timings_index();
>> -     dssdev->panel.timings = cea_vesa_timings[code].timings;
>> +     copy_hdmi_to_dss_timings(cea_vesa_timings[code].timings,
>> +                     &dssdev->panel.timings);
>> +
>>       update_hdmi_timings(&hdmi.cfg, p, code);
>>
>>       phy = p->pixel_clock;
>> diff --git a/drivers/video/omap2/dss/hdmi.h
>> b/drivers/video/omap2/dss/hdmi.h
>> index f57ef4a..dcc30b7 100644
>> --- a/drivers/video/omap2/dss/hdmi.h
>> +++ b/drivers/video/omap2/dss/hdmi.h
>> @@ -188,9 +188,22 @@ struct hdmi_reg { u16 idx; };
>>  #define REG_GET(base, idx, start, end) \
>>       FLD_GET(hdmi_read_reg(base, idx), start, end)
>>
>> +struct hdmi_video_timings {
>> +     u16 x_res;
>> +     u16 y_res;
>> +     /* Unit: KHz */
>> +     u32 pixel_clock;
>
> [sp] Could be:
>        u32 pixel_clock; /* unit: KHz */
>
>> +     u16 hsw;
>> +     u16 hfp;
>> +     u16 hbp;
>> +     u16 vsw;
>> +     u16 vfp;
>> +     u16 vbp;
>> +};
>> +
>>  /* HDMI timing structure */
>>  struct hdmi_timings {
>> -     struct omap_video_timings timings;
>> +     struct hdmi_video_timings timings;
>>       int vsync_pol;
>>       int hsync_pol;
>>  };
>> --
>> 1.7.5.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
Tomi Valkeinen June 23, 2011, 8:30 a.m. UTC | #3
On Fri, 2011-06-17 at 13:47 +0530, Mythri P K wrote:
> Define new HDMI timings structure to replace the OMAP DSS timing strucutre in hdmi.c
> to have the HDMI include defintion out of DSS.
> 
> Signed-off-by: Mythri P K <mythripk@ti.com>
> ---
>  drivers/video/omap2/dss/hdmi.c |   23 ++++++++++++++++++++---
>  drivers/video/omap2/dss/hdmi.h |   15 ++++++++++++++-
>  2 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index 4ee879a..c24d573 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -593,6 +593,20 @@ static int read_edid(struct hdmi_ip_data *ip_data, u8 *pedid, u16 max_length)
>  	return 0;
>  }
>  
> +static void copy_hdmi_to_dss_timings(struct hdmi_video_timings hdmi_timings,
> +			struct omap_video_timings *timings)

There's no reason to pass hdmi_timings as a value, so it should be a
pointer.

> +{
> +	timings->x_res = hdmi_timings.x_res;
> +	timings->y_res = hdmi_timings.y_res;
> +	timings->pixel_clock = hdmi_timings.pixel_clock;
> +	timings->hbp = hdmi_timings.hbp;
> +	timings->hfp = hdmi_timings.hfp;
> +	timings->hsw = hdmi_timings.hsw;
> +	timings->vbp = hdmi_timings.vbp;
> +	timings->vfp = hdmi_timings.vfp;
> +	timings->vsw = hdmi_timings.vsw;
> +}
> +
>  static int get_timings_index(void)
>  {
>  	int code;
> @@ -617,7 +631,7 @@ static struct hdmi_cm hdmi_get_code(struct omap_video_timings *timing)
>  {
>  	int i = 0, code = -1, temp_vsync = 0, temp_hsync = 0;
>  	int timing_vsync = 0, timing_hsync = 0;
> -	struct omap_video_timings temp;
> +	struct hdmi_video_timings temp;
>  	struct hdmi_cm cm = {-1};
>  	DSSDBG("hdmi_get_code\n");
>  
> @@ -775,7 +789,8 @@ static void hdmi_read_edid(struct omap_video_timings *dp)

"dp" feels a rather odd name for video timings variable. What does it
stand for?

 Tomi


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
K, Mythri P June 23, 2011, 8:46 a.m. UTC | #4
Hi Tomi,

On Thu, Jun 23, 2011 at 2:00 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Fri, 2011-06-17 at 13:47 +0530, Mythri P K wrote:
>> Define new HDMI timings structure to replace the OMAP DSS timing strucutre in hdmi.c
>> to have the HDMI include defintion out of DSS.
>>
>> Signed-off-by: Mythri P K <mythripk@ti.com>
>> ---
>>  drivers/video/omap2/dss/hdmi.c |   23 ++++++++++++++++++++---
>>  drivers/video/omap2/dss/hdmi.h |   15 ++++++++++++++-
>>  2 files changed, 34 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
>> index 4ee879a..c24d573 100644
>> --- a/drivers/video/omap2/dss/hdmi.c
>> +++ b/drivers/video/omap2/dss/hdmi.c
>> @@ -593,6 +593,20 @@ static int read_edid(struct hdmi_ip_data *ip_data, u8 *pedid, u16 max_length)
>>       return 0;
>>  }
>>
>> +static void copy_hdmi_to_dss_timings(struct hdmi_video_timings hdmi_timings,
>> +                     struct omap_video_timings *timings)
>
> There's no reason to pass hdmi_timings as a value, so it should be a
> pointer.
As the hdmi_timings is not changed in the function , i passed it by
value, Not a problem to change.
>
>> +{
>> +     timings->x_res = hdmi_timings.x_res;
>> +     timings->y_res = hdmi_timings.y_res;
>> +     timings->pixel_clock = hdmi_timings.pixel_clock;
>> +     timings->hbp = hdmi_timings.hbp;
>> +     timings->hfp = hdmi_timings.hfp;
>> +     timings->hsw = hdmi_timings.hsw;
>> +     timings->vbp = hdmi_timings.vbp;
>> +     timings->vfp = hdmi_timings.vfp;
>> +     timings->vsw = hdmi_timings.vsw;
>> +}
>> +
>>  static int get_timings_index(void)
>>  {
>>       int code;
>> @@ -617,7 +631,7 @@ static struct hdmi_cm hdmi_get_code(struct omap_video_timings *timing)
>>  {
>>       int i = 0, code = -1, temp_vsync = 0, temp_hsync = 0;
>>       int timing_vsync = 0, timing_hsync = 0;
>> -     struct omap_video_timings temp;
>> +     struct hdmi_video_timings temp;
>>       struct hdmi_cm cm = {-1};
>>       DSSDBG("hdmi_get_code\n");
>>
>> @@ -775,7 +789,8 @@ static void hdmi_read_edid(struct omap_video_timings *dp)
>
> "dp" feels a rather odd name for video timings variable. What does it
> stand for?

*display pointer, but then it is not a part of this patch.

>
>  Tomi
>
>
>
diff mbox

Patch

diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 4ee879a..c24d573 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -593,6 +593,20 @@  static int read_edid(struct hdmi_ip_data *ip_data, u8 *pedid, u16 max_length)
 	return 0;
 }
 
+static void copy_hdmi_to_dss_timings(struct hdmi_video_timings hdmi_timings,
+			struct omap_video_timings *timings)
+{
+	timings->x_res = hdmi_timings.x_res;
+	timings->y_res = hdmi_timings.y_res;
+	timings->pixel_clock = hdmi_timings.pixel_clock;
+	timings->hbp = hdmi_timings.hbp;
+	timings->hfp = hdmi_timings.hfp;
+	timings->hsw = hdmi_timings.hsw;
+	timings->vbp = hdmi_timings.vbp;
+	timings->vfp = hdmi_timings.vfp;
+	timings->vsw = hdmi_timings.vsw;
+}
+
 static int get_timings_index(void)
 {
 	int code;
@@ -617,7 +631,7 @@  static struct hdmi_cm hdmi_get_code(struct omap_video_timings *timing)
 {
 	int i = 0, code = -1, temp_vsync = 0, temp_hsync = 0;
 	int timing_vsync = 0, timing_hsync = 0;
-	struct omap_video_timings temp;
+	struct hdmi_video_timings temp;
 	struct hdmi_cm cm = {-1};
 	DSSDBG("hdmi_get_code\n");
 
@@ -775,7 +789,8 @@  static void hdmi_read_edid(struct omap_video_timings *dp)
 
 	code = get_timings_index();
 
-	*dp = cea_vesa_timings[code].timings;
+	copy_hdmi_to_dss_timings(cea_vesa_timings[code].timings, dp);
+
 }
 
 static void hdmi_core_init(struct hdmi_core_video_config *video_cfg,
@@ -1234,7 +1249,9 @@  static int hdmi_power_on(struct omap_dss_device *dssdev)
 		hdmi_read_edid(p);
 	}
 	code = get_timings_index();
-	dssdev->panel.timings = cea_vesa_timings[code].timings;
+	copy_hdmi_to_dss_timings(cea_vesa_timings[code].timings,
+			&dssdev->panel.timings);
+
 	update_hdmi_timings(&hdmi.cfg, p, code);
 
 	phy = p->pixel_clock;
diff --git a/drivers/video/omap2/dss/hdmi.h b/drivers/video/omap2/dss/hdmi.h
index f57ef4a..dcc30b7 100644
--- a/drivers/video/omap2/dss/hdmi.h
+++ b/drivers/video/omap2/dss/hdmi.h
@@ -188,9 +188,22 @@  struct hdmi_reg { u16 idx; };
 #define REG_GET(base, idx, start, end) \
 	FLD_GET(hdmi_read_reg(base, idx), start, end)
 
+struct hdmi_video_timings {
+	u16 x_res;
+	u16 y_res;
+	/* Unit: KHz */
+	u32 pixel_clock;
+	u16 hsw;
+	u16 hfp;
+	u16 hbp;
+	u16 vsw;
+	u16 vfp;
+	u16 vbp;
+};
+
 /* HDMI timing structure */
 struct hdmi_timings {
-	struct omap_video_timings timings;
+	struct hdmi_video_timings timings;
 	int vsync_pol;
 	int hsync_pol;
 };