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(-)
Fixes wrong picture offset observed when using HDMI output with a Technisat HD TV.
Signed-off-by: Lucas Stach dev@lynxeye.de Acked-by: Mark Zhang markz@nvidia.com Tested-by: Mark Zhang markz@nvidia.com --- 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) {
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@lynxeye.de --- 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; }
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@lynxeye.de --- 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;
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@lynxeye.de
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;
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
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@lynxeye.de --- 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) |
There is no gem.c anymore, those functions are implemented by the drm_cma_helpers now.
Signed-off-by: Lucas Stach dev@lynxeye.de --- 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);
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@lynxeye.de
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);
The intention is to program exactly WIN_A, not WIN_A and possibly others.
Signed-off-by: Lucas Stach dev@lynxeye.de --- 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);
dri-devel@lists.freedesktop.org
-
Lucas Stach -
Mark Zhang