[PATCH 0/6] tegradrm fixes and cleanups

[PATCH 0/6] tegradrm fixes and cleanups

[Lucas Stach]

Some small fixes and cleanups for the tegradrm driver.
This is potentially 3.8-fixes material.

Lucas Stach (6):
  drm: tegra: fix front_porch <-> back_porch mixup
  drm: tegra: don't leave clients host1x member uninitialized
  drm: tegra: protect DC register access with mutex
  drm: tegra: remove redundant tegra2_tmds_config entry
  drm: tegra: clean out old gem prototypes
  drm: tegra: program only one window during modeset

 drivers/gpu/drm/tegra/dc.c     | 24 ++++++++++++++++++------
 drivers/gpu/drm/tegra/drm.h    | 19 +------------------
 drivers/gpu/drm/tegra/hdmi.c   | 25 ++++++-------------------
 drivers/gpu/drm/tegra/host1x.c |  2 ++
 4 Dateien geändert, 27 Zeilen hinzugefügt(+), 43 Zeilen entfernt(-)

-- 
1.7.11.7

[PATCH 1/6] drm: tegra: fix front_porch <-> back_porch mixup

[Lucas Stach]

Fixes wrong picture offset observed when using HDMI output with a
Technisat HD TV.

Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
Acked-by: Mark Zhang <markz-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Tested-by: Mark Zhang <markz-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Captions are a bit confusing here. As the porch is always relative to
the sync pulse, the left picture margin is actually the back_porch.
---
 drivers/gpu/drm/tegra/dc.c   | 8 ++++----
 drivers/gpu/drm/tegra/hdmi.c | 6 +++---
 2 Dateien geändert, 7 Zeilen hinzugefügt(+), 7 Zeilen entfernt(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 0744103..54683e4 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -102,12 +102,12 @@ static int tegra_dc_set_timings(struct tegra_dc *dc,
 		((mode->hsync_end - mode->hsync_start) <<  0);
 	tegra_dc_writel(dc, value, DC_DISP_SYNC_WIDTH);
 
-	value = ((mode->vsync_start - mode->vdisplay) << 16) |
-		((mode->hsync_start - mode->hdisplay) <<  0);
-	tegra_dc_writel(dc, value, DC_DISP_BACK_PORCH);
-
 	value = ((mode->vtotal - mode->vsync_end) << 16) |
 		((mode->htotal - mode->hsync_end) <<  0);
+	tegra_dc_writel(dc, value, DC_DISP_BACK_PORCH);
+
+	value = ((mode->vsync_start - mode->vdisplay) << 16) |
+		((mode->hsync_start - mode->hdisplay) <<  0);
 	tegra_dc_writel(dc, value, DC_DISP_FRONT_PORCH);
 
 	value = (mode->vdisplay << 16) | mode->hdisplay;
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index ab40164..81ea934 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -479,7 +479,7 @@ static void tegra_hdmi_setup_avi_infoframe(struct tegra_hdmi *hdmi,
 		return;
 	}
 
-	h_front_porch = mode->htotal - mode->hsync_end;
+	h_front_porch = mode->hsync_start - mode->hdisplay;
 	memset(&frame, 0, sizeof(frame));
 	frame.r = HDMI_AVI_R_SAME;
 
@@ -634,8 +634,8 @@ static int tegra_output_hdmi_enable(struct tegra_output *output)
 
 	pclk = mode->clock * 1000;
 	h_sync_width = mode->hsync_end - mode->hsync_start;
-	h_front_porch = mode->htotal - mode->hsync_end;
-	h_back_porch = mode->hsync_start - mode->hdisplay;
+	h_back_porch = mode->htotal - mode->hsync_end;
+	h_front_porch = mode->hsync_start - mode->hdisplay;
 
 	err = regulator_enable(hdmi->vdd);
 	if (err < 0) {
-- 
1.7.11.7

[PATCH 2/6] drm: tegra: don't leave clients host1x member uninitialized

[Lucas Stach]

No real problem for now, as nothing is using this, but leaving it
unitialized is asking for trouble later on.

Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
 drivers/gpu/drm/tegra/host1x.c | 2 ++
 1 Datei geändert, 2 Zeilen hinzugefügt(+)

diff --git a/drivers/gpu/drm/tegra/host1x.c b/drivers/gpu/drm/tegra/host1x.c
index bdb97a5..5d17b11 100644
--- a/drivers/gpu/drm/tegra/host1x.c
+++ b/drivers/gpu/drm/tegra/host1x.c
@@ -239,6 +239,8 @@ int host1x_register_client(struct host1x *host1x, struct host1x_client *client)
 		}
 	}
 
+	client->host1x = host1x;
+
 	return 0;
 }
 
-- 
1.7.11.7

[PATCH 3/6] drm: tegra: protect DC register access with mutex

[Lucas Stach]

Window properties are programmed through a shared aperture and have to
happen atomically. Also we do the read-update-write dance on some of the
shared regs.
To make sure that different functions don't stumble over each other
protect the register access with a mutex.

Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
We could probably make this a bit more fine grained, but this would add
some complexity and I don't really see a win there for now.
---
 drivers/gpu/drm/tegra/dc.c  | 13 +++++++++++++
 drivers/gpu/drm/tegra/drm.h |  1 +
 2 Dateien geändert, 14 Zeilen hinzugefügt(+)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 54683e4..b256574 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -171,6 +171,8 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
 		return err;
 	}
 
+	mutex_lock(&dc->regs_mutex);
+
 	/* program display mode */
 	tegra_dc_set_timings(dc, mode);
 
@@ -269,6 +271,8 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
 	tegra_dc_writel(dc, 0xff00, DC_WIN_BLEND_NOKEY);
 	tegra_dc_writel(dc, 0xff00, DC_WIN_BLEND_1WIN);
 
+	mutex_unlock(&dc->regs_mutex);
+
 	return 0;
 }
 
@@ -287,6 +291,8 @@ static void tegra_crtc_prepare(struct drm_crtc *crtc)
 	else
 		syncpt = SYNCPT_VBLANK0;
 
+	mutex_lock(&dc->regs_mutex);
+
 	/* initialize display controller */
 	tegra_dc_writel(dc, 0x00000100, DC_CMD_GENERAL_INCR_SYNCPT_CNTRL);
 	tegra_dc_writel(dc, 0x100 | syncpt, DC_CMD_CONT_SYNCPT_VSYNC);
@@ -320,6 +326,8 @@ static void tegra_crtc_prepare(struct drm_crtc *crtc)
 
 	value = VBLANK_INT | WIN_A_UF_INT | WIN_B_UF_INT | WIN_C_UF_INT;
 	tegra_dc_writel(dc, value, DC_CMD_INT_ENABLE);
+
+	mutex_unlock(&dc->regs_mutex);
 }
 
 static void tegra_crtc_commit(struct drm_crtc *crtc)
@@ -330,6 +338,8 @@ static void tegra_crtc_commit(struct drm_crtc *crtc)
 
 	update_mask = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
 
+	mutex_lock(&dc->regs_mutex);
+
 	tegra_dc_writel(dc, update_mask << 8, DC_CMD_STATE_CONTROL);
 
 	value = tegra_dc_readl(dc, DC_CMD_INT_ENABLE);
@@ -341,6 +351,8 @@ static void tegra_crtc_commit(struct drm_crtc *crtc)
 	tegra_dc_writel(dc, value, DC_CMD_INT_MASK);
 
 	tegra_dc_writel(dc, update_mask, DC_CMD_STATE_CONTROL);
+
+	mutex_unlock(&dc->regs_mutex);
 }
 
 static void tegra_crtc_load_lut(struct drm_crtc *crtc)
@@ -747,6 +759,7 @@ static int tegra_dc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	INIT_LIST_HEAD(&dc->list);
+	mutex_init(&dc->regs_mutex);
 	dc->dev = &pdev->dev;
 
 	dc->clk = devm_clk_get(&pdev->dev, NULL);
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 3a843a7..eae1f56 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -84,6 +84,7 @@ struct tegra_dc {
 
 	struct clk *clk;
 
+	struct mutex regs_mutex;
 	void __iomem *regs;
 	int irq;
 
-- 
1.7.11.7

[PATCH 4/6] drm: tegra: remove redundant tegra2_tmds_config entry

[Lucas Stach]

The 720p and 1080p entries are completely redundant, as we are matching
the table entries against <=pclk.
Also generalize the comment, as we are using those table entries even
when driving other modes than the standard TV ones.

Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
 drivers/gpu/drm/tegra/hdmi.c | 19 +++----------------
 1 Datei geändert, 3 Zeilen hinzugefügt(+), 16 Zeilen entfernt(-)

diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index 81ea934..e060c7e 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -149,7 +149,7 @@ struct tmds_config {
 };
 
 static const struct tmds_config tegra2_tmds_config[] = {
-	{ /* 480p modes */
+	{ /* slow pixel clock modes */
 		.pclk = 27000000,
 		.pll0 = SOR_PLL_BG_V17_S(3) | SOR_PLL_ICHPMP(1) |
 			SOR_PLL_RESISTORSEL | SOR_PLL_VCOCAP(0) |
@@ -163,21 +163,8 @@ static const struct tmds_config tegra2_tmds_config[] = {
 			DRIVE_CURRENT_LANE1(DRIVE_CURRENT_7_125_mA) |
 			DRIVE_CURRENT_LANE2(DRIVE_CURRENT_7_125_mA) |
 			DRIVE_CURRENT_LANE3(DRIVE_CURRENT_7_125_mA),
-	}, { /* 720p modes */
-		.pclk = 74250000,
-		.pll0 = SOR_PLL_BG_V17_S(3) | SOR_PLL_ICHPMP(1) |
-			SOR_PLL_RESISTORSEL | SOR_PLL_VCOCAP(1) |
-			SOR_PLL_TX_REG_LOAD(3),
-		.pll1 = SOR_PLL_TMDS_TERM_ENABLE | SOR_PLL_PE_EN,
-		.pe_current = PE_CURRENT0(PE_CURRENT_6_0_mA) |
-			PE_CURRENT1(PE_CURRENT_6_0_mA) |
-			PE_CURRENT2(PE_CURRENT_6_0_mA) |
-			PE_CURRENT3(PE_CURRENT_6_0_mA),
-		.drive_current = DRIVE_CURRENT_LANE0(DRIVE_CURRENT_7_125_mA) |
-			DRIVE_CURRENT_LANE1(DRIVE_CURRENT_7_125_mA) |
-			DRIVE_CURRENT_LANE2(DRIVE_CURRENT_7_125_mA) |
-			DRIVE_CURRENT_LANE3(DRIVE_CURRENT_7_125_mA),
-	}, { /* 1080p modes */
+	},
+	{ /* high pixel clock modes */
 		.pclk = UINT_MAX,
 		.pll0 = SOR_PLL_BG_V17_S(3) | SOR_PLL_ICHPMP(1) |
 			SOR_PLL_RESISTORSEL | SOR_PLL_VCOCAP(1) |
-- 
1.7.11.7

[PATCH 5/6] drm: tegra: clean out old gem prototypes

[Lucas Stach]

There is no gem.c anymore, those functions are implemented by the
drm_cma_helpers now.

Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
 drivers/gpu/drm/tegra/drm.h | 18 ------------------
 1 Datei geändert, 18 Zeilen entfernt(-)

diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index eae1f56..71e61f2 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -205,24 +205,6 @@ extern int tegra_output_parse_dt(struct tegra_output *output);
 extern int tegra_output_init(struct drm_device *drm, struct tegra_output *output);
 extern int tegra_output_exit(struct tegra_output *output);
 
-/* from gem.c */
-extern struct tegra_gem_object *tegra_gem_alloc(struct drm_device *drm,
-						size_t size);
-extern int tegra_gem_handle_create(struct drm_device *drm,
-				   struct drm_file *file, size_t size,
-				   unsigned long flags, uint32_t *handle);
-extern int tegra_gem_dumb_create(struct drm_file *file, struct drm_device *drm,
-				 struct drm_mode_create_dumb *args);
-extern int tegra_gem_dumb_map_offset(struct drm_file *file,
-				     struct drm_device *drm, uint32_t handle,
-				     uint64_t *offset);
-extern int tegra_gem_dumb_destroy(struct drm_file *file,
-				  struct drm_device *drm, uint32_t handle);
-extern int tegra_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
-extern int tegra_gem_init_object(struct drm_gem_object *obj);
-extern void tegra_gem_free_object(struct drm_gem_object *obj);
-extern struct vm_operations_struct tegra_gem_vm_ops;
-
 /* from fb.c */
 extern int tegra_drm_fb_init(struct drm_device *drm);
 extern void tegra_drm_fb_exit(struct drm_device *drm);
-- 
1.7.11.7

[PATCH 6/6] drm: tegra: program only one window during modeset

[Lucas Stach]

The intention is to program exactly WIN_A, not WIN_A and possibly
others.

Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
 drivers/gpu/drm/tegra/dc.c | 3 +--
 1 Datei geändert, 1 Zeile hinzugefügt(+), 2 Zeilen entfernt(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index b256574..3475bd9 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -223,8 +223,7 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
 	win.stride = crtc->fb->pitches[0];
 
 	/* program window registers */
-	value = tegra_dc_readl(dc, DC_CMD_DISPLAY_WINDOW_HEADER);
-	value |= WINDOW_A_SELECT;
+	value = WINDOW_A_SELECT;
 	tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER);
 
 	tegra_dc_writel(dc, win.fmt, DC_WIN_COLOR_DEPTH);
-- 
1.7.11.7

Re: [PATCH 3/6] drm: tegra: protect DC register access with mutex

[Mark Zhang]

OK, you add a mutex in a tegra_dc structure. But I think there is no
parallel scenario while we operate on a dc. AFAIK, the functions which
you add mutex protection are called by drm sequentially(except for
function "tegra_crtc_load_lut" I'm not very clear about that).

So could you give us an example?

Mark
On 12/20/2012 05:38 AM, Lucas Stach wrote:
> Window properties are programmed through a shared aperture and have to
> happen atomically. Also we do the read-update-write dance on some of the
> shared regs.
> To make sure that different functions don't stumble over each other
> protect the register access with a mutex.
> 
> Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
> ---
> We could probably make this a bit more fine grained, but this would add
> some complexity and I don't really see a win there for now.
> ---
>  drivers/gpu/drm/tegra/dc.c  | 13 +++++++++++++
>  drivers/gpu/drm/tegra/drm.h |  1 +
>  2 Dateien geändert, 14 Zeilen hinzugefügt(+)
> 
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 54683e4..b256574 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -171,6 +171,8 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
>  		return err;
>  	}
>  
> +	mutex_lock(&dc->regs_mutex);
> +
>  	/* program display mode */
>  	tegra_dc_set_timings(dc, mode);
>  
> @@ -269,6 +271,8 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
>  	tegra_dc_writel(dc, 0xff00, DC_WIN_BLEND_NOKEY);
>  	tegra_dc_writel(dc, 0xff00, DC_WIN_BLEND_1WIN);
>  
> +	mutex_unlock(&dc->regs_mutex);
> +
>  	return 0;
>  }
>  
> @@ -287,6 +291,8 @@ static void tegra_crtc_prepare(struct drm_crtc *crtc)
>  	else
>  		syncpt = SYNCPT_VBLANK0;
>  
> +	mutex_lock(&dc->regs_mutex);
> +
>  	/* initialize display controller */
>  	tegra_dc_writel(dc, 0x00000100, DC_CMD_GENERAL_INCR_SYNCPT_CNTRL);
>  	tegra_dc_writel(dc, 0x100 | syncpt, DC_CMD_CONT_SYNCPT_VSYNC);
> @@ -320,6 +326,8 @@ static void tegra_crtc_prepare(struct drm_crtc *crtc)
>  
>  	value = VBLANK_INT | WIN_A_UF_INT | WIN_B_UF_INT | WIN_C_UF_INT;
>  	tegra_dc_writel(dc, value, DC_CMD_INT_ENABLE);
> +
> +	mutex_unlock(&dc->regs_mutex);
>  }
>  
>  static void tegra_crtc_commit(struct drm_crtc *crtc)
> @@ -330,6 +338,8 @@ static void tegra_crtc_commit(struct drm_crtc *crtc)
>  
>  	update_mask = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
>  
> +	mutex_lock(&dc->regs_mutex);
> +
>  	tegra_dc_writel(dc, update_mask << 8, DC_CMD_STATE_CONTROL);
>  
>  	value = tegra_dc_readl(dc, DC_CMD_INT_ENABLE);
> @@ -341,6 +351,8 @@ static void tegra_crtc_commit(struct drm_crtc *crtc)
>  	tegra_dc_writel(dc, value, DC_CMD_INT_MASK);
>  
>  	tegra_dc_writel(dc, update_mask, DC_CMD_STATE_CONTROL);
> +
> +	mutex_unlock(&dc->regs_mutex);
>  }
>  
>  static void tegra_crtc_load_lut(struct drm_crtc *crtc)
> @@ -747,6 +759,7 @@ static int tegra_dc_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	INIT_LIST_HEAD(&dc->list);
> +	mutex_init(&dc->regs_mutex);
>  	dc->dev = &pdev->dev;
>  
>  	dc->clk = devm_clk_get(&pdev->dev, NULL);
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index 3a843a7..eae1f56 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -84,6 +84,7 @@ struct tegra_dc {
>  
>  	struct clk *clk;
>  
> +	struct mutex regs_mutex;
>  	void __iomem *regs;
>  	int irq;
>  
> 

Re: [PATCH 5/6] drm: tegra: clean out old gem prototypes

[Mark Zhang]

We must forget about that. I believe no one read the header files while
review. So thanks. :)

Mark
On 12/20/2012 05:38 AM, Lucas Stach wrote:
> There is no gem.c anymore, those functions are implemented by the
> drm_cma_helpers now.
> 
> Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
> ---
>  drivers/gpu/drm/tegra/drm.h | 18 ------------------
>  1 Datei geändert, 18 Zeilen entfernt(-)
> 
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index eae1f56..71e61f2 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -205,24 +205,6 @@ extern int tegra_output_parse_dt(struct tegra_output *output);
>  extern int tegra_output_init(struct drm_device *drm, struct tegra_output *output);
>  extern int tegra_output_exit(struct tegra_output *output);
>  
> -/* from gem.c */
> -extern struct tegra_gem_object *tegra_gem_alloc(struct drm_device *drm,
> -						size_t size);
> -extern int tegra_gem_handle_create(struct drm_device *drm,
> -				   struct drm_file *file, size_t size,
> -				   unsigned long flags, uint32_t *handle);
> -extern int tegra_gem_dumb_create(struct drm_file *file, struct drm_device *drm,
> -				 struct drm_mode_create_dumb *args);
> -extern int tegra_gem_dumb_map_offset(struct drm_file *file,
> -				     struct drm_device *drm, uint32_t handle,
> -				     uint64_t *offset);
> -extern int tegra_gem_dumb_destroy(struct drm_file *file,
> -				  struct drm_device *drm, uint32_t handle);
> -extern int tegra_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
> -extern int tegra_gem_init_object(struct drm_gem_object *obj);
> -extern void tegra_gem_free_object(struct drm_gem_object *obj);
> -extern struct vm_operations_struct tegra_gem_vm_ops;
> -
>  /* from fb.c */
>  extern int tegra_drm_fb_init(struct drm_device *drm);
>  extern void tegra_drm_fb_exit(struct drm_device *drm);
> 

Re: [PATCH 3/6] drm: tegra: protect DC register access with mutex

[Lucas Stach]

Am Donnerstag, den 20.12.2012, 10:36 +0800 schrieb Mark Zhang:
> OK, you add a mutex in a tegra_dc structure. But I think there is no
> parallel scenario while we operate on a dc. AFAIK, the functions which
> you add mutex protection are called by drm sequentially(except for
> function "tegra_crtc_load_lut" I'm not very clear about that).
> 
> So could you give us an example?
> 
You are right, I looked this up again. I thought cursor actions are
async to other modeset actions, but in fact they are also protected by
the mode_config.mutex. So really no need to have another mutex in our dc
code.

Sorry for the noise.

Lucas