All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/3] drm: bridge/dw_hdmi: fixed codec style
       [not found] <425876114-22392-1-git-send-email-ykk@rock-chips.com>
@ 2015-03-09  4:42 ` Yakir Yang
  2015-03-09  4:48   ` Joe Perches
  2015-03-09  7:05     ` Daniel Kurtz
  2015-03-09  4:42 ` [PATCH v2 2/3] drm: bridge/dw_hdmi_rockchip: improve hdmi eye-diagram test Yakir Yang
  2015-03-09  4:42 ` [PATCH v2 3/3] drm: bridge/dw_hdmi: improve hdmi single-end test Yakir Yang
  2 siblings, 2 replies; 10+ messages in thread
From: Yakir Yang @ 2015-03-09  4:42 UTC (permalink / raw)
  To: David Airlie, Russell King, Philipp Zabel, djkurtz
  Cc: Fabio Estevam, Shawn Guo, Rob Clark, Mark Yao, Daniel Vetter,
	dri-devel, linux-kernel, Andy Yan, mmind00, dianders, marcheu,
	linux-rockchip, Yakir Yang

- const struct dw_hdmi_mpll_config *mpll_config =
-                                        hdmi->plat_data->mpll_cfg;
- const struct dw_hdmi_curr_ctrl *curr_ctrl = hdmi->plat_data->cur_ctr;
- const struct dw_hdmi_sym_term *sym_term =  hdmi->plat_data->sym_term;

+ const struct dw_hdmi_plat_data *plat_data = hdmi->plat_data;
+ const struct dw_hdmi_mpll_config *mpll_config = plat_data->mpll_cfg;
+ const struct dw_hdmi_curr_ctrl *curr_ctrl = plat_data->cur_ctr;
+ const struct dw_hdmi_sym_term *sym_term =  plat_data->sym_term;

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---

Changes in v2: None

 drivers/gpu/drm/bridge/dw_hdmi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index 9ffc257..b9d8d8a 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -900,10 +900,10 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, unsigned char prep,
 {
 	unsigned res_idx, i;
 	u8 val, msec;
-	const struct dw_hdmi_mpll_config *mpll_config =
-						hdmi->plat_data->mpll_cfg;
-	const struct dw_hdmi_curr_ctrl *curr_ctrl = hdmi->plat_data->cur_ctr;
-	const struct dw_hdmi_sym_term *sym_term =  hdmi->plat_data->sym_term;
+	const struct dw_hdmi_plat_data *plat_data = hdmi->plat_data;
+	const struct dw_hdmi_mpll_config *mpll_config = plat_data->mpll_cfg;
+	const struct dw_hdmi_curr_ctrl *curr_ctrl = plat_data->cur_ctr;
+	const struct dw_hdmi_sym_term *sym_term =  plat_data->sym_term;
 
 	if (prep)
 		return -EINVAL;
-- 
2.1.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/3] drm: bridge/dw_hdmi_rockchip: improve hdmi eye-diagram test
       [not found] <425876114-22392-1-git-send-email-ykk@rock-chips.com>
  2015-03-09  4:42 ` [PATCH v2 1/3] drm: bridge/dw_hdmi: fixed codec style Yakir Yang
@ 2015-03-09  4:42 ` Yakir Yang
  2015-03-09  4:42 ` [PATCH v2 3/3] drm: bridge/dw_hdmi: improve hdmi single-end test Yakir Yang
  2 siblings, 0 replies; 10+ messages in thread
From: Yakir Yang @ 2015-03-09  4:42 UTC (permalink / raw)
  To: David Airlie, Russell King, Philipp Zabel, djkurtz
  Cc: Fabio Estevam, Shawn Guo, Rob Clark, Mark Yao, Daniel Vetter,
	dri-devel, linux-kernel, Andy Yan, mmind00, dianders, marcheu,
	linux-rockchip, Yakir Yang

As for 1920x1080 display resolution, we should turn on the
Transmitter Trailer-B.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---

Changes in v2:
- Set slopeboost back to 10%-20%, then rasing/falling time would pass.

 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index e46e1d9..be0371d 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -142,7 +142,7 @@ static const struct dw_hdmi_curr_ctrl rockchip_cur_ctr[] = {
 static const struct dw_hdmi_sym_term rockchip_sym_term[] = {
 	/*pixelclk   symbol   term*/
 	{ 74250000,  0x8009, 0x0004 },
-	{ 148500000, 0x8029, 0x0004 },
+	{ 148500000, 0x802b, 0x0004 },
 	{ 297000000, 0x8039, 0x0005 },
 	{ ~0UL,	     0x0000, 0x0000 }
 };
-- 
2.1.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] drm: bridge/dw_hdmi: improve hdmi single-end test
       [not found] <425876114-22392-1-git-send-email-ykk@rock-chips.com>
  2015-03-09  4:42 ` [PATCH v2 1/3] drm: bridge/dw_hdmi: fixed codec style Yakir Yang
  2015-03-09  4:42 ` [PATCH v2 2/3] drm: bridge/dw_hdmi_rockchip: improve hdmi eye-diagram test Yakir Yang
@ 2015-03-09  4:42 ` Yakir Yang
  2 siblings, 0 replies; 10+ messages in thread
From: Yakir Yang @ 2015-03-09  4:42 UTC (permalink / raw)
  To: David Airlie, Russell King, Philipp Zabel, djkurtz
  Cc: Fabio Estevam, Shawn Guo, Rob Clark, Mark Yao, Daniel Vetter,
	dri-devel, linux-kernel, Andy Yan, mmind00, dianders, marcheu,
	linux-rockchip, Yakir Yang

Because of iMX6 & Rockchip have differnet mpll config parameter,
than the cklvl & txlvl would be different, we also should seperate
this parmeter.

As for Rockchip HDMI, when pixle clock less than 148MHz, the cklvl &
txlvl should set to 13. When pixel clock less than 74.25MHz the cklvl
should set to 18, txlvl should set to 19.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---

Changes in v2:
- For pixel clock less than 74.25MHz, set txlvl to 19 and set cklvl to 18.

 drivers/gpu/drm/bridge/dw_hdmi.c            | 14 +++++++-------
 drivers/gpu/drm/imx/dw_hdmi-imx.c           | 12 ++++++------
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 14 +++++++-------
 include/drm/bridge/dw_hdmi.h                |  5 +++--
 4 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index b9d8d8a..6edefdd 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -903,7 +903,7 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, unsigned char prep,
 	const struct dw_hdmi_plat_data *plat_data = hdmi->plat_data;
 	const struct dw_hdmi_mpll_config *mpll_config = plat_data->mpll_cfg;
 	const struct dw_hdmi_curr_ctrl *curr_ctrl = plat_data->cur_ctr;
-	const struct dw_hdmi_sym_term *sym_term =  plat_data->sym_term;
+	const struct dw_hdmi_phy_config *phy_config = plat_data->phy_config;
 
 	if (prep)
 		return -EINVAL;
@@ -974,18 +974,18 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, unsigned char prep,
 	hdmi_phy_i2c_write(hdmi, 0x0000, 0x13);  /* PLLPHBYCTRL */
 	hdmi_phy_i2c_write(hdmi, 0x0006, 0x17);
 
-	for (i = 0; sym_term[i].mpixelclock != (~0UL); i++)
+	for (i = 0; phy_config[i].mpixelclock != (~0UL); i++)
 		if (hdmi->hdmi_data.video_mode.mpixelclock <=
-		    sym_term[i].mpixelclock)
+		    phy_config[i].mpixelclock)
 			break;
 
 	/* RESISTANCE TERM 133Ohm Cfg */
-	hdmi_phy_i2c_write(hdmi, sym_term[i].term, 0x19);  /* TXTERM */
+	hdmi_phy_i2c_write(hdmi, phy_config[i].term, 0x19);  /* TXTERM */
 	/* PREEMP Cgf 0.00 */
-	hdmi_phy_i2c_write(hdmi, sym_term[i].sym_ctr, 0x09);  /* CKSYMTXCTRL */
-
+	hdmi_phy_i2c_write(hdmi, phy_config[i].sym_ctr, 0x09); /* CKSYMTXCTRL */
 	/* TX/CK LVL 10 */
-	hdmi_phy_i2c_write(hdmi, 0x01ad, 0x0E);  /* VLEVCTRL */
+	hdmi_phy_i2c_write(hdmi, phy_config[i].vlev_ctr, 0x0E); /* VLEVCTRL */
+
 	/* REMOVE CLK TERM */
 	hdmi_phy_i2c_write(hdmi, 0x8000, 0x05);  /* CKCALCTRL */
 
diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index 121d30c..d6095d2 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -73,10 +73,10 @@ static const struct dw_hdmi_curr_ctrl imx_cur_ctr[] = {
 	}
 };
 
-static const struct dw_hdmi_sym_term imx_sym_term[] = {
-	/*pixelclk   symbol   term*/
-	{ 148500000, 0x800d, 0x0005 },
-	{ ~0UL,      0x0000, 0x0000 }
+static const struct dw_hdmi_phy_config imx_phy_config[] = {
+	/*pixelclk   symbol   term   vlev */
+	{ 148500000, 0x800d, 0x0005, 0x01ad},
+	{ ~0UL,      0x0000, 0x0000, 0x0000}
 };
 
 static int dw_hdmi_imx_parse_dt(struct imx_hdmi *hdmi)
@@ -139,14 +139,14 @@ static struct drm_encoder_funcs dw_hdmi_imx_encoder_funcs = {
 static struct dw_hdmi_plat_data imx6q_hdmi_drv_data = {
 	.mpll_cfg = imx_mpll_cfg,
 	.cur_ctr  = imx_cur_ctr,
-	.sym_term = imx_sym_term,
+	.phy_config = imx_phy_config,
 	.dev_type = IMX6Q_HDMI,
 };
 
 static struct dw_hdmi_plat_data imx6dl_hdmi_drv_data = {
 	.mpll_cfg = imx_mpll_cfg,
 	.cur_ctr  = imx_cur_ctr,
-	.sym_term = imx_sym_term,
+	.phy_config = imx_phy_config,
 	.dev_type = IMX6DL_HDMI,
 };
 
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index be0371d..9b29a6f 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -139,12 +139,12 @@ static const struct dw_hdmi_curr_ctrl rockchip_cur_ctr[] = {
 	}
 };
 
-static const struct dw_hdmi_sym_term rockchip_sym_term[] = {
-	/*pixelclk   symbol   term*/
-	{ 74250000,  0x8009, 0x0004 },
-	{ 148500000, 0x802b, 0x0004 },
-	{ 297000000, 0x8039, 0x0005 },
-	{ ~0UL,	     0x0000, 0x0000 }
+static const struct dw_hdmi_phy_config rockchip_phy_config[] = {
+	/*pixelclk   symbol   term   vlev*/
+	{ 74250000,  0x8009, 0x0004, 0x0272},
+	{ 148500000, 0x802b, 0x0004, 0x01ad},
+	{ 297000000, 0x8039, 0x0005, 0x01ad},
+	{ ~0UL,	     0x0000, 0x0000, 0x0000}
 };
 
 static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
@@ -236,7 +236,7 @@ static const struct dw_hdmi_plat_data rockchip_hdmi_drv_data = {
 	.mode_valid = dw_hdmi_rockchip_mode_valid,
 	.mpll_cfg   = rockchip_mpll_cfg,
 	.cur_ctr    = rockchip_cur_ctr,
-	.sym_term   = rockchip_sym_term,
+	.phy_config = rockchip_phy_config,
 	.dev_type   = RK3288_HDMI,
 };
 
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index fb07a90..8e9885d 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -40,10 +40,11 @@ struct dw_hdmi_curr_ctrl {
 	u16 curr[DW_HDMI_RES_MAX];
 };
 
-struct dw_hdmi_sym_term {
+struct dw_hdmi_phy_config {
 	unsigned long mpixelclock;
 	u16 sym_ctr;    /*clock symbol and transmitter control*/
 	u16 term;       /*transmission termination value*/
+	u16 vlev_ctr;   /* voltage level control */
 };
 
 struct dw_hdmi_audio_data {
@@ -63,7 +64,7 @@ struct dw_hdmi_plat_data {
 	enum dw_hdmi_devtype dev_type;
 	const struct dw_hdmi_mpll_config *mpll_cfg;
 	const struct dw_hdmi_curr_ctrl *cur_ctr;
-	const struct dw_hdmi_sym_term *sym_term;
+	const struct dw_hdmi_phy_config *phy_config;
 	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
 					   struct drm_display_mode *mode);
 };
-- 
2.1.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] drm: bridge/dw_hdmi: fixed codec style
  2015-03-09  4:42 ` [PATCH v2 1/3] drm: bridge/dw_hdmi: fixed codec style Yakir Yang
@ 2015-03-09  4:48   ` Joe Perches
  2015-03-09  6:48       ` Joe Perches
  2015-03-09  7:05     ` Daniel Kurtz
  1 sibling, 1 reply; 10+ messages in thread
From: Joe Perches @ 2015-03-09  4:48 UTC (permalink / raw)
  To: Yakir Yang
  Cc: David Airlie, Russell King, Philipp Zabel, djkurtz,
	Fabio Estevam, Shawn Guo, Rob Clark, Mark Yao, Daniel Vetter,
	dri-devel, linux-kernel, Andy Yan, mmind00, dianders, marcheu,
	linux-rockchip

On Mon, 2015-03-09 at 12:42 +0800, Yakir Yang wrote:
> diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
[]
> @@ -900,10 +900,10 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, unsigned char prep,
>  {
>  	unsigned res_idx, i;
>  	u8 val, msec;
> -	const struct dw_hdmi_mpll_config *mpll_config =
> -						hdmi->plat_data->mpll_cfg;
> -	const struct dw_hdmi_curr_ctrl *curr_ctrl = hdmi->plat_data->cur_ctr;
> -	const struct dw_hdmi_sym_term *sym_term =  hdmi->plat_data->sym_term;
> +	const struct dw_hdmi_plat_data *plat_data = hdmi->plat_data;
> +	const struct dw_hdmi_mpll_config *mpll_config = plat_data->mpll_cfg;
> +	const struct dw_hdmi_curr_ctrl *curr_ctrl = plat_data->cur_ctr;
> +	const struct dw_hdmi_sym_term *sym_term =  plat_data->sym_term;
>  
>  	if (prep)
>  		return -EINVAL;

Shouldn't all of these be static?



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] drm: bridge/dw_hdmi: fixed codec style
  2015-03-09  4:48   ` Joe Perches
@ 2015-03-09  6:48       ` Joe Perches
  0 siblings, 0 replies; 10+ messages in thread
From: Joe Perches @ 2015-03-09  6:48 UTC (permalink / raw)
  To: Yakir Yang
  Cc: David Airlie, Russell King, Philipp Zabel, djkurtz,
	Fabio Estevam, Shawn Guo, Rob Clark, Mark Yao, Daniel Vetter,
	dri-devel, linux-kernel, Andy Yan, mmind00, dianders, marcheu,
	linux-rockchip

On Sun, 2015-03-08 at 21:48 -0700, Joe Perches wrote:

> Shouldn't all of these be static?

Don't mind me.  These shouldn't be static.

I was a bit mislead by the commit message.

I think it'd be better not to put patch-like
+ and - lines in the commit description.

cheers, Joe


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] drm: bridge/dw_hdmi: fixed codec style
@ 2015-03-09  6:48       ` Joe Perches
  0 siblings, 0 replies; 10+ messages in thread
From: Joe Perches @ 2015-03-09  6:48 UTC (permalink / raw)
  To: Yakir Yang
  Cc: Fabio Estevam, mmind00, marcheu, linux-kernel, dianders,
	linux-rockchip, dri-devel, Andy Yan, Russell King

On Sun, 2015-03-08 at 21:48 -0700, Joe Perches wrote:

> Shouldn't all of these be static?

Don't mind me.  These shouldn't be static.

I was a bit mislead by the commit message.

I think it'd be better not to put patch-like
+ and - lines in the commit description.

cheers, Joe

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] drm: bridge/dw_hdmi: fixed codec style
  2015-03-09  4:42 ` [PATCH v2 1/3] drm: bridge/dw_hdmi: fixed codec style Yakir Yang
@ 2015-03-09  7:05     ` Daniel Kurtz
  2015-03-09  7:05     ` Daniel Kurtz
  1 sibling, 0 replies; 10+ messages in thread
From: Daniel Kurtz @ 2015-03-09  7:05 UTC (permalink / raw)
  To: Yakir Yang
  Cc: David Airlie, Russell King, Philipp Zabel, Fabio Estevam,
	Heiko Stübner, linux-kernel, dri-devel, Douglas Anderson,
	open list:ARM/Rockchip SoC...,
	Rob Clark, Daniel Vetter, Stéphane Marchesin, Andy Yan,
	Shawn Guo, Mark Yao

On Mon, Mar 9, 2015 at 12:42 PM, Yakir Yang <ykk@rock-chips.com> wrote:
> - const struct dw_hdmi_mpll_config *mpll_config =
> -                                        hdmi->plat_data->mpll_cfg;
> - const struct dw_hdmi_curr_ctrl *curr_ctrl = hdmi->plat_data->cur_ctr;
> - const struct dw_hdmi_sym_term *sym_term =  hdmi->plat_data->sym_term;
>
> + const struct dw_hdmi_plat_data *plat_data = hdmi->plat_data;
> + const struct dw_hdmi_mpll_config *mpll_config = plat_data->mpll_cfg;
> + const struct dw_hdmi_curr_ctrl *curr_ctrl = plat_data->cur_ctr;
> + const struct dw_hdmi_sym_term *sym_term =  plat_data->sym_term;
>
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>

I agree with Joe Perches that this commit message is not very clear.
It should summarize what the patches is doing and why, not just be a
copy of the change.
e.g., "Using a local struct pointer to reduce one level of indirection
makes the code slightly more readable."

, but otherwise this is:
Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>

> ---
>
> Changes in v2: None
>
>  drivers/gpu/drm/bridge/dw_hdmi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
> index 9ffc257..b9d8d8a 100644
> --- a/drivers/gpu/drm/bridge/dw_hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw_hdmi.c
> @@ -900,10 +900,10 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, unsigned char prep,
>  {
>         unsigned res_idx, i;
>         u8 val, msec;
> -       const struct dw_hdmi_mpll_config *mpll_config =
> -                                               hdmi->plat_data->mpll_cfg;
> -       const struct dw_hdmi_curr_ctrl *curr_ctrl = hdmi->plat_data->cur_ctr;
> -       const struct dw_hdmi_sym_term *sym_term =  hdmi->plat_data->sym_term;
> +       const struct dw_hdmi_plat_data *plat_data = hdmi->plat_data;
> +       const struct dw_hdmi_mpll_config *mpll_config = plat_data->mpll_cfg;
> +       const struct dw_hdmi_curr_ctrl *curr_ctrl = plat_data->cur_ctr;
> +       const struct dw_hdmi_sym_term *sym_term =  plat_data->sym_term;
>
>         if (prep)
>                 return -EINVAL;
> --
> 2.1.2
>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] drm: bridge/dw_hdmi: fixed codec style
@ 2015-03-09  7:05     ` Daniel Kurtz
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Kurtz @ 2015-03-09  7:05 UTC (permalink / raw)
  To: Yakir Yang
  Cc: Heiko Stübner, Fabio Estevam, linux-kernel, dri-devel,
	Douglas Anderson, open list:ARM/Rockchip SoC...,
	Stéphane Marchesin, Russell King, Andy Yan

On Mon, Mar 9, 2015 at 12:42 PM, Yakir Yang <ykk@rock-chips.com> wrote:
> - const struct dw_hdmi_mpll_config *mpll_config =
> -                                        hdmi->plat_data->mpll_cfg;
> - const struct dw_hdmi_curr_ctrl *curr_ctrl = hdmi->plat_data->cur_ctr;
> - const struct dw_hdmi_sym_term *sym_term =  hdmi->plat_data->sym_term;
>
> + const struct dw_hdmi_plat_data *plat_data = hdmi->plat_data;
> + const struct dw_hdmi_mpll_config *mpll_config = plat_data->mpll_cfg;
> + const struct dw_hdmi_curr_ctrl *curr_ctrl = plat_data->cur_ctr;
> + const struct dw_hdmi_sym_term *sym_term =  plat_data->sym_term;
>
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>

I agree with Joe Perches that this commit message is not very clear.
It should summarize what the patches is doing and why, not just be a
copy of the change.
e.g., "Using a local struct pointer to reduce one level of indirection
makes the code slightly more readable."

, but otherwise this is:
Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>

> ---
>
> Changes in v2: None
>
>  drivers/gpu/drm/bridge/dw_hdmi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
> index 9ffc257..b9d8d8a 100644
> --- a/drivers/gpu/drm/bridge/dw_hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw_hdmi.c
> @@ -900,10 +900,10 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, unsigned char prep,
>  {
>         unsigned res_idx, i;
>         u8 val, msec;
> -       const struct dw_hdmi_mpll_config *mpll_config =
> -                                               hdmi->plat_data->mpll_cfg;
> -       const struct dw_hdmi_curr_ctrl *curr_ctrl = hdmi->plat_data->cur_ctr;
> -       const struct dw_hdmi_sym_term *sym_term =  hdmi->plat_data->sym_term;
> +       const struct dw_hdmi_plat_data *plat_data = hdmi->plat_data;
> +       const struct dw_hdmi_mpll_config *mpll_config = plat_data->mpll_cfg;
> +       const struct dw_hdmi_curr_ctrl *curr_ctrl = plat_data->cur_ctr;
> +       const struct dw_hdmi_sym_term *sym_term =  plat_data->sym_term;
>
>         if (prep)
>                 return -EINVAL;
> --
> 2.1.2
>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] drm: bridge/dw_hdmi: fixed codec style
  2015-03-09  6:48       ` Joe Perches
  (?)
@ 2015-03-09  7:23       ` yakir
  -1 siblings, 0 replies; 10+ messages in thread
From: yakir @ 2015-03-09  7:23 UTC (permalink / raw)
  To: Joe Perches
  Cc: David Airlie, Russell King, Philipp Zabel, djkurtz,
	Fabio Estevam, Shawn Guo, Rob Clark, Mark Yao, Daniel Vetter,
	dri-devel, linux-kernel, Andy Yan, mmind00, dianders, marcheu,
	linux-rockchip


On 2015年03月09日 14:48, Joe Perches wrote:
> On Sun, 2015-03-08 at 21:48 -0700, Joe Perches wrote:
>
>> Shouldn't all of these be static?
> Don't mind me.  These shouldn't be static.
>
> I was a bit mislead by the commit message.
>
> I think it'd be better not to put patch-like
> + and - lines in the commit description.
>
> cheers, Joe
>
>
Okay, I will delete it from commit message.

Thanks for reply :)

>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] drm: bridge/dw_hdmi: fixed codec style
  2015-03-09  7:05     ` Daniel Kurtz
  (?)
@ 2015-03-09  7:25     ` yakir
  -1 siblings, 0 replies; 10+ messages in thread
From: yakir @ 2015-03-09  7:25 UTC (permalink / raw)
  To: Daniel Kurtz
  Cc: David Airlie, Russell King, Philipp Zabel, Fabio Estevam,
	Heiko Stübner, linux-kernel, dri-devel, Douglas Anderson,
	open list:ARM/Rockchip SoC...,
	Rob Clark, Daniel Vetter, Stéphane Marchesin, Andy Yan,
	Shawn Guo, Mark Yao


On 2015年03月09日 15:05, Daniel Kurtz wrote:
> On Mon, Mar 9, 2015 at 12:42 PM, Yakir Yang <ykk@rock-chips.com> wrote:
>> - const struct dw_hdmi_mpll_config *mpll_config =
>> -                                        hdmi->plat_data->mpll_cfg;
>> - const struct dw_hdmi_curr_ctrl *curr_ctrl = hdmi->plat_data->cur_ctr;
>> - const struct dw_hdmi_sym_term *sym_term =  hdmi->plat_data->sym_term;
>>
>> + const struct dw_hdmi_plat_data *plat_data = hdmi->plat_data;
>> + const struct dw_hdmi_mpll_config *mpll_config = plat_data->mpll_cfg;
>> + const struct dw_hdmi_curr_ctrl *curr_ctrl = plat_data->cur_ctr;
>> + const struct dw_hdmi_sym_term *sym_term =  plat_data->sym_term;
>>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> I agree with Joe Perches that this commit message is not very clear.
> It should summarize what the patches is doing and why, not just be a
> copy of the change.
> e.g., "Using a local struct pointer to reduce one level of indirection
> makes the code slightly more readable."
>
> , but otherwise this is:
> Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>

Okay, I will correct it now.

Thanks for your reivew  :)

>> ---
>>
>> Changes in v2: None
>>
>>   drivers/gpu/drm/bridge/dw_hdmi.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
>> index 9ffc257..b9d8d8a 100644
>> --- a/drivers/gpu/drm/bridge/dw_hdmi.c
>> +++ b/drivers/gpu/drm/bridge/dw_hdmi.c
>> @@ -900,10 +900,10 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, unsigned char prep,
>>   {
>>          unsigned res_idx, i;
>>          u8 val, msec;
>> -       const struct dw_hdmi_mpll_config *mpll_config =
>> -                                               hdmi->plat_data->mpll_cfg;
>> -       const struct dw_hdmi_curr_ctrl *curr_ctrl = hdmi->plat_data->cur_ctr;
>> -       const struct dw_hdmi_sym_term *sym_term =  hdmi->plat_data->sym_term;
>> +       const struct dw_hdmi_plat_data *plat_data = hdmi->plat_data;
>> +       const struct dw_hdmi_mpll_config *mpll_config = plat_data->mpll_cfg;
>> +       const struct dw_hdmi_curr_ctrl *curr_ctrl = plat_data->cur_ctr;
>> +       const struct dw_hdmi_sym_term *sym_term =  plat_data->sym_term;
>>
>>          if (prep)
>>                  return -EINVAL;
>> --
>> 2.1.2
>>
>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-03-09  7:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <425876114-22392-1-git-send-email-ykk@rock-chips.com>
2015-03-09  4:42 ` [PATCH v2 1/3] drm: bridge/dw_hdmi: fixed codec style Yakir Yang
2015-03-09  4:48   ` Joe Perches
2015-03-09  6:48     ` Joe Perches
2015-03-09  6:48       ` Joe Perches
2015-03-09  7:23       ` yakir
2015-03-09  7:05   ` Daniel Kurtz
2015-03-09  7:05     ` Daniel Kurtz
2015-03-09  7:25     ` yakir
2015-03-09  4:42 ` [PATCH v2 2/3] drm: bridge/dw_hdmi_rockchip: improve hdmi eye-diagram test Yakir Yang
2015-03-09  4:42 ` [PATCH v2 3/3] drm: bridge/dw_hdmi: improve hdmi single-end test Yakir Yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.