Re: [PATCH] drm/exynos: hdmi: using drm_display_mode timings for exynos4

2013-02-27 Thread Sean Paul
On Fri, Feb 22, 2013 at 8:32 AM, Rahul Sharma rahul.sha...@samsung.com wrote:
 Exynos5 is already using drm_display_mode for timings parameters. Exynos4
 is also modifed to use the same. List of supported resolutions and
 corresponding timings are removed which helps is enabling some extra
 resolutions. It also cleans some of the duplicate code.


Cool!

 Exynos4 and Exynos5 Mixers, work fine for the same range of resolutions. Hence
 same condition (to find the supported mode) is applied to both.

 More exynos4 phy configs can be added later to extend the mode supprot.

 Signed-off-by: Rahul Sharma rahul.sha...@samsung.com
 ---
 It is based on exynos-drm-next-todo branch at
 git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git

  drivers/gpu/drm/exynos/exynos_hdmi.c  | 659 
 +-
  drivers/gpu/drm/exynos/exynos_mixer.c |   4 -
  2 files changed, 246 insertions(+), 417 deletions(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
 b/drivers/gpu/drm/exynos/exynos_hdmi.c
 index 273a6ea..09e82a0 100644
 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
 +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
 @@ -109,7 +109,20 @@ struct hdmi_tg_regs {
 u8 tg_3d[1];
  };

 -struct hdmi_core_regs {
 +struct hdmi_v13_core_regs {
 +   u8 h_blank[2];
 +   u8 v_blank[3];
 +   u8 h_v_line[3];
 +   u8 vsync_pol[1];
 +   u8 int_pro_mode[1];
 +   u8 v_blank_f[3];
 +   u8 h_sync_gen[3];
 +   u8 v_sync_gen1[3];
 +   u8 v_sync_gen2[3];
 +   u8 v_sync_gen3[3];
 +};

If you're going to do this refactor, I think you should try to merge
the various v13/v14 structures. They're pretty close as-is and this
would cut a bunch of duplicate code out of the driver.

Sean

 +
 +struct hdmi_v14_core_regs {
 u8 h_blank[2];
 u8 v2_blank[2];
 u8 v1_blank[2];
 @@ -148,11 +161,23 @@ struct hdmi_core_regs {
 u8 vact_space_6[2];
  };

 +struct hdmi_v13_conf {
 +   struct hdmi_v13_core_regs core;
 +   struct hdmi_tg_regs tg;
 +};
 +
  struct hdmi_v14_conf {
 -   int pixel_clock;
 -   struct hdmi_core_regs core;
 +   struct hdmi_v14_core_regs core;
 struct hdmi_tg_regs tg;
 +};
 +
 +struct hdmi_conf_regs {
 +   int pixel_clock;
 int cea_video_id;
 +   union {
 +   struct hdmi_v13_conf v13_conf;
 +   struct hdmi_v14_conf v14_conf;
 +   } conf;
  };

  struct hdmi_context {
 @@ -171,9 +196,8 @@ struct hdmi_context {
 struct i2c_client   *ddc_port;
 struct i2c_client   *hdmiphy_port;

 -   /* current hdmiphy conf index */
 -   int cur_conf;
 -   struct hdmi_v14_confmode_conf;
 +   /* current hdmiphy conf regs */
 +   struct hdmi_conf_regs   mode_conf;

 struct hdmi_resources   res;

 @@ -182,292 +206,60 @@ struct hdmi_context {
 enum hdmi_type  type;
  };

 -/* HDMI Version 1.3 */
 -static const u8 hdmiphy_v13_conf27[32] = {
 -   0x01, 0x05, 0x00, 0xD8, 0x10, 0x1C, 0x30, 0x40,
 -   0x6B, 0x10, 0x02, 0x51, 0xDF, 0xF2, 0x54, 0x87,
 -   0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0,
 -   0x22, 0x40, 0xE3, 0x26, 0x00, 0x00, 0x00, 0x00,
 -};
 -
 -static const u8 hdmiphy_v13_conf27_027[32] = {
 -   0x01, 0x05, 0x00, 0xD4, 0x10, 0x9C, 0x09, 0x64,
 -   0x6B, 0x10, 0x02, 0x51, 0xDF, 0xF2, 0x54, 0x87,
 -   0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0,
 -   0x22, 0x40, 0xE3, 0x26, 0x00, 0x00, 0x00, 0x00,
 -};
 -
 -static const u8 hdmiphy_v13_conf74_175[32] = {
 -   0x01, 0x05, 0x00, 0xD8, 0x10, 0x9C, 0xef, 0x5B,
 -   0x6D, 0x10, 0x01, 0x51, 0xef, 0xF3, 0x54, 0xb9,
 -   0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0,
 -   0x22, 0x40, 0xa5, 0x26, 0x01, 0x00, 0x00, 0x00,
 -};
 -
 -static const u8 hdmiphy_v13_conf74_25[32] = {
 -   0x01, 0x05, 0x00, 0xd8, 0x10, 0x9c, 0xf8, 0x40,
 -   0x6a, 0x10, 0x01, 0x51, 0xff, 0xf1, 0x54, 0xba,
 -   0x84, 0x00, 0x10, 0x38, 0x00, 0x08, 0x10, 0xe0,
 -   0x22, 0x40, 0xa4, 0x26, 0x01, 0x00, 0x00, 0x00,
 -};
 -
 -static const u8 hdmiphy_v13_conf148_5[32] = {
 -   0x01, 0x05, 0x00, 0xD8, 0x10, 0x9C, 0xf8, 0x40,
 -   0x6A, 0x18, 0x00, 0x51, 0xff, 0xF1, 0x54, 0xba,
 -   0x84, 0x00, 0x10, 0x38, 0x00, 0x08, 0x10, 0xE0,
 -   0x22, 0x40, 0xa4, 0x26, 0x02, 0x00, 0x00, 0x00,
 -};
 -
 -struct hdmi_v13_tg_regs {
 -   u8 cmd;
 -   u8 h_fsz_l;
 -   u8 h_fsz_h;
 -   u8 hact_st_l;
 -   u8 hact_st_h;
 -   u8 hact_sz_l;
 -   u8 hact_sz_h;
 -   u8 v_fsz_l;
 -   u8 v_fsz_h;
 -   u8 vsync_l;
 -   u8 vsync_h;
 -   u8 vsync2_l;
 -   u8 vsync2_h;
 -   u8 vact_st_l;
 -   u8 vact_st_h;
 -   u8 vact_sz_l;
 -   u8 vact_sz_h;
 -   u8 field_chg_l;
 -   u8 field_chg_h;
 -   u8 vact_st2_l;
 -   u8 vact_st2_h;
 -   u8 vsync_top_hdmi_l;
 -   u8 vsync_top_hdmi_h;
 -   u8 vsync_bot_hdmi_l;
 -  

Re: [PATCH] drm/exynos: hdmi: using drm_display_mode timings for exynos4

2013-02-26 Thread Rahul Sharma
Thanks Sean,

On Tue, Feb 26, 2013 at 11:33 PM, Sean Paul seanp...@google.com wrote:
 On Fri, Feb 22, 2013 at 8:32 AM, Rahul Sharma rahul.sha...@samsung.com 
 wrote:
 Exynos5 is already using drm_display_mode for timings parameters. Exynos4
 is also modifed to use the same. List of supported resolutions and
 corresponding timings are removed which helps is enabling some extra
 resolutions. It also cleans some of the duplicate code.


 Cool!

 Exynos4 and Exynos5 Mixers, work fine for the same range of resolutions. 
 Hence
 same condition (to find the supported mode) is applied to both.

 More exynos4 phy configs can be added later to extend the mode supprot.

 Signed-off-by: Rahul Sharma rahul.sha...@samsung.com
 ---
 It is based on exynos-drm-next-todo branch at
 git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git

  drivers/gpu/drm/exynos/exynos_hdmi.c  | 659 
 +-
  drivers/gpu/drm/exynos/exynos_mixer.c |   4 -
  2 files changed, 246 insertions(+), 417 deletions(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
 b/drivers/gpu/drm/exynos/exynos_hdmi.c
 index 273a6ea..09e82a0 100644
 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
 +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
 @@ -109,7 +109,20 @@ struct hdmi_tg_regs {
 u8 tg_3d[1];
  };

 -struct hdmi_core_regs {
 +struct hdmi_v13_core_regs {
 +   u8 h_blank[2];
 +   u8 v_blank[3];
 +   u8 h_v_line[3];
 +   u8 vsync_pol[1];
 +   u8 int_pro_mode[1];
 +   u8 v_blank_f[3];
 +   u8 h_sync_gen[3];
 +   u8 v_sync_gen1[3];
 +   u8 v_sync_gen2[3];
 +   u8 v_sync_gen3[3];
 +};

 If you're going to do this refactor, I think you should try to merge
 the various v13/v14 structures. They're pretty close as-is and this
 would cut a bunch of duplicate code out of the driver.

 Sean


I have tried to merge them. I kept common hdmi_tg_regs
for v13 and v14 (v14 TG regs are extension to v13 TG regs).
But hdmi_core_regs fields are one to one mapped with Hdmi
Core Register which are bit different in both the versions.

Additionally, using common hdmi_conf_regs which have unions
for each hw version. A new version can be seamlessly added
to the list.

Please tell me specific region for improvements.

regards,
Rahul Sharma.

 +
 +struct hdmi_v14_core_regs {
 u8 h_blank[2];
 u8 v2_blank[2];
 u8 v1_blank[2];
 @@ -148,11 +161,23 @@ struct hdmi_core_regs {
 u8 vact_space_6[2];
  };

 +struct hdmi_v13_conf {
 +   struct hdmi_v13_core_regs core;
 +   struct hdmi_tg_regs tg;
 +};
 +
  struct hdmi_v14_conf {
 -   int pixel_clock;
 -   struct hdmi_core_regs core;
 +   struct hdmi_v14_core_regs core;
 struct hdmi_tg_regs tg;
 +};
 +
 +struct hdmi_conf_regs {
 +   int pixel_clock;
 int cea_video_id;
 +   union {
 +   struct hdmi_v13_conf v13_conf;
 +   struct hdmi_v14_conf v14_conf;
 +   } conf;
  };

  struct hdmi_context {
 @@ -171,9 +196,8 @@ struct hdmi_context {
 struct i2c_client   *ddc_port;
 struct i2c_client   *hdmiphy_port;

 -   /* current hdmiphy conf index */
 -   int cur_conf;
 -   struct hdmi_v14_confmode_conf;
 +   /* current hdmiphy conf regs */
 +   struct hdmi_conf_regs   mode_conf;

 struct hdmi_resources   res;

 @@ -182,292 +206,60 @@ struct hdmi_context {
 enum hdmi_type  type;
  };

 -/* HDMI Version 1.3 */
 -static const u8 hdmiphy_v13_conf27[32] = {
 -   0x01, 0x05, 0x00, 0xD8, 0x10, 0x1C, 0x30, 0x40,
 -   0x6B, 0x10, 0x02, 0x51, 0xDF, 0xF2, 0x54, 0x87,
 -   0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0,
 -   0x22, 0x40, 0xE3, 0x26, 0x00, 0x00, 0x00, 0x00,
 -};
 -
 -static const u8 hdmiphy_v13_conf27_027[32] = {
 -   0x01, 0x05, 0x00, 0xD4, 0x10, 0x9C, 0x09, 0x64,
 -   0x6B, 0x10, 0x02, 0x51, 0xDF, 0xF2, 0x54, 0x87,
 -   0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0,
 -   0x22, 0x40, 0xE3, 0x26, 0x00, 0x00, 0x00, 0x00,
 -};
 -
 -static const u8 hdmiphy_v13_conf74_175[32] = {
 -   0x01, 0x05, 0x00, 0xD8, 0x10, 0x9C, 0xef, 0x5B,
 -   0x6D, 0x10, 0x01, 0x51, 0xef, 0xF3, 0x54, 0xb9,
 -   0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0,
 -   0x22, 0x40, 0xa5, 0x26, 0x01, 0x00, 0x00, 0x00,
 -};
 -
 -static const u8 hdmiphy_v13_conf74_25[32] = {
 -   0x01, 0x05, 0x00, 0xd8, 0x10, 0x9c, 0xf8, 0x40,
 -   0x6a, 0x10, 0x01, 0x51, 0xff, 0xf1, 0x54, 0xba,
 -   0x84, 0x00, 0x10, 0x38, 0x00, 0x08, 0x10, 0xe0,
 -   0x22, 0x40, 0xa4, 0x26, 0x01, 0x00, 0x00, 0x00,
 -};
 -
 -static const u8 hdmiphy_v13_conf148_5[32] = {
 -   0x01, 0x05, 0x00, 0xD8, 0x10, 0x9C, 0xf8, 0x40,
 -   0x6A, 0x18, 0x00, 0x51, 0xff, 0xF1, 0x54, 0xba,
 -   0x84, 0x00, 0x10, 0x38, 0x00, 0x08, 0x10, 0xE0,
 -   0x22, 0x40, 0xa4, 0x26, 0x02, 0x00, 0x00, 0x00,
 -};
 -
 -struct hdmi_v13_tg_regs {
 -   u8 cmd;
 -