This patch series introduces a number of useful features for the Tegra DRM driver. Patch 1 is a documentation update and adds a warning to the page-flip IOCTL to catch buggy drivers that return success but failed to update the crtc->fb field, which would cause the reference counting to become unbalanced.
Patch 2 adds support for the additional two planes available on Tegra hardware, while patch 3 implement .mode_set_base() to allow for some nice speed up when changing the framebuffer without actually changing the resolution. Patch 4 adds VBLANK support and patch 5 builds on the previous two to provide the page-flipping IOCTL. Patch 6 splits the DC_CMD_STATE_CONTROL register writes into two consecutive writes as required by the TRM.
Finally patch 7 adds a new file, named "framebuffers" to debugfs which can be used to dump a list of framebuffers attached to the DRM device. This is most useful to inspect the reference count but could also be helpful in diagnosing out-of-memory conditions and such.
Thierry
Thierry Reding (7): drm: Add consistency check for page-flipping drm/tegra: Add plane support drm/tegra: Implement .mode_set_base() drm/tegra: Implement VBLANK support drm/tegra: Implement page-flipping support drm/tegra: Split DC_CMD_STATE_CONTROL register write drm/tegra: Add list of framebuffers to debugfs
Documentation/DocBook/drm.tmpl | 6 + drivers/gpu/drm/drm_crtc.c | 7 + drivers/gpu/drm/tegra/dc.c | 489 ++++++++++++++++++++++++++++++++--------- drivers/gpu/drm/tegra/dc.h | 2 +- drivers/gpu/drm/tegra/drm.c | 103 +++++++++ drivers/gpu/drm/tegra/drm.h | 37 ++++ 6 files changed, 533 insertions(+), 111 deletions(-)
Driver implementations of the drm_crtc's .page_flip() function are required to update the crtc->fb field on success to reflect that the new framebuffer is now in use. This is important to keep reference counting on the framebuffers balanced.
While at it, document this requirement to keep others from falling into the same trap.
Suggested-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Thierry Reding thierry.reding@avionic-design.de --- Documentation/DocBook/drm.tmpl | 6 ++++++ drivers/gpu/drm/drm_crtc.c | 7 +++++++ 2 files changed, 13 insertions(+)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index fae8018..8dfaeb0 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -1161,6 +1161,12 @@ int max_width, max_height;</synopsis> any new rendering to the frame buffer until the page flip completes. </para> <para> + If a page flip can be successfully scheduled the driver must set the + <code>drm_crtc-<fb</code> field to the new framebuffer pointed to + by <code>fb</code>. This is important so that the reference counting + on framebuffers stays balanced. + </para> + <para> If a page flip is already pending, the <methodname>page_flip</methodname> operation must return -<errorname>EBUSY</errorname>. diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 838c9b6..d86edc1 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3766,6 +3766,13 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, /* Keep the old fb, don't unref it. */ old_fb = NULL; } else { + /* + * Warn if the driver hasn't properly updated the crtc->fb + * field to reflect that the new framebuffer is now used. + * Failing to do so will screw with the reference counting + * on framebuffers. + */ + WARN_ON(crtc->fb != fb); /* Unref only the old framebuffer. */ fb = NULL; }
On Wed, Feb 13, 2013 at 05:05:00PM +0100, Thierry Reding wrote:
Driver implementations of the drm_crtc's .page_flip() function are required to update the crtc->fb field on success to reflect that the new framebuffer is now in use. This is important to keep reference counting on the framebuffers balanced.
While at it, document this requirement to keep others from falling into the same trap.
Suggested-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Thierry Reding thierry.reding@avionic-design.de
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Documentation/DocBook/drm.tmpl | 6 ++++++ drivers/gpu/drm/drm_crtc.c | 7 +++++++ 2 files changed, 13 insertions(+)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index fae8018..8dfaeb0 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -1161,6 +1161,12 @@ int max_width, max_height;</synopsis> any new rendering to the frame buffer until the page flip completes. </para> <para>
If a page flip can be successfully scheduled the driver must set the<code>drm_crtc-<fb</code> field to the new framebuffer pointed toby <code>fb</code>. This is important so that the reference countingon framebuffers stays balanced.</para><para> If a page flip is already pending, the <methodname>page_flip</methodname> operation must return -<errorname>EBUSY</errorname>.diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 838c9b6..d86edc1 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3766,6 +3766,13 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, /* Keep the old fb, don't unref it. */ old_fb = NULL; } else {
/** Warn if the driver hasn't properly updated the crtc->fb* field to reflect that the new framebuffer is now used.* Failing to do so will screw with the reference counting* on framebuffers.*/WARN_ON(crtc->fb != fb);-- 1.8.1.2
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Add support for the B and C planes which support RGB and YUV pixel formats and can be used as overlays or hardware cursor. Currently only 32-bit RGBA pixel formats are advertised.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de --- Changes in v3: - split DC_CMD_STATE_CONTROL writes - adjust plane index to match window index - drop support for YUV422 format for now - disable active planes when CRTC is disabled - disable plane on module unload
drivers/gpu/drm/tegra/dc.c | 338 ++++++++++++++++++++++++++++++++------------ drivers/gpu/drm/tegra/dc.h | 2 +- drivers/gpu/drm/tegra/drm.h | 29 ++++ 3 files changed, 274 insertions(+), 95 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index b6679b3..8f97b1c 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -17,26 +17,124 @@ #include "drm.h" #include "dc.h"
-struct tegra_dc_window { - fixed20_12 x; - fixed20_12 y; - fixed20_12 w; - fixed20_12 h; - unsigned int outx; - unsigned int outy; - unsigned int outw; - unsigned int outh; - unsigned int stride; - unsigned int fmt; +struct tegra_plane { + struct drm_plane base; + unsigned int index; };
+static inline struct tegra_plane *to_tegra_plane(struct drm_plane *plane) +{ + return container_of(plane, struct tegra_plane, base); +} + +static int tegra_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, + struct drm_framebuffer *fb, int crtc_x, + int crtc_y, unsigned int crtc_w, + unsigned int crtc_h, uint32_t src_x, + uint32_t src_y, uint32_t src_w, uint32_t src_h) +{ + unsigned long base = tegra_framebuffer_base(fb); + struct tegra_plane *p = to_tegra_plane(plane); + struct tegra_dc *dc = to_tegra_dc(crtc); + struct tegra_dc_window window; + + memset(&window, 0, sizeof(window)); + window.src.x = src_x >> 16; + window.src.y = src_y >> 16; + window.src.w = src_w >> 16; + window.src.h = src_h >> 16; + window.dst.x = crtc_x; + window.dst.y = crtc_y; + window.dst.w = crtc_w; + window.dst.h = crtc_h; + window.format = tegra_dc_format(fb->pixel_format); + window.bits_per_pixel = fb->bits_per_pixel; + window.stride = fb->pitches[0]; + window.base = base; + + return tegra_dc_setup_window(dc, p->index, &window); +} + +static int tegra_plane_disable(struct drm_plane *plane) +{ + struct tegra_dc *dc = to_tegra_dc(plane->crtc); + struct tegra_plane *p = to_tegra_plane(plane); + unsigned long value; + + value = WINDOW_A_SELECT << p->index; + tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER); + + value = tegra_dc_readl(dc, DC_WIN_WIN_OPTIONS); + value &= ~WIN_ENABLE; + tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS); + + tegra_dc_writel(dc, WIN_A_UPDATE << p->index, DC_CMD_STATE_CONTROL); + tegra_dc_writel(dc, WIN_A_ACT_REQ << p->index, DC_CMD_STATE_CONTROL); + + return 0; +} + +static void tegra_plane_destroy(struct drm_plane *plane) +{ + tegra_plane_disable(plane); + drm_plane_cleanup(plane); +} + +static const struct drm_plane_funcs tegra_plane_funcs = { + .update_plane = tegra_plane_update, + .disable_plane = tegra_plane_disable, + .destroy = tegra_plane_destroy, +}; + +static const uint32_t plane_formats[] = { + DRM_FORMAT_XRGB8888, +}; + +static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc) +{ + unsigned int i; + int err = 0; + + for (i = 0; i < 2; i++) { + struct tegra_plane *plane; + + plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL); + if (!plane) + return -ENOMEM; + + plane->index = 1 + i; + + err = drm_plane_init(drm, &plane->base, 1 << dc->pipe, + &tegra_plane_funcs, plane_formats, + ARRAY_SIZE(plane_formats), false); + if (err < 0) + return err; + } + + return 0; +} + static const struct drm_crtc_funcs tegra_crtc_funcs = { .set_config = drm_crtc_helper_set_config, .destroy = drm_crtc_cleanup, };
-static void tegra_crtc_dpms(struct drm_crtc *crtc, int mode) +static void tegra_crtc_disable(struct drm_crtc *crtc) { + struct drm_device *drm = crtc->dev; + struct drm_plane *plane; + + list_for_each_entry(plane, &drm->mode_config.plane_list, head) { + if (plane->crtc == crtc) { + tegra_plane_disable(plane); + plane->crtc = NULL; + + if (plane->fb) { + drm_framebuffer_unreference(plane->fb); + plane->fb = NULL; + } + } + } }
static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc, @@ -46,10 +144,11 @@ static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc, return true; }
-static inline u32 compute_dda_inc(fixed20_12 inf, unsigned int out, bool v, +static inline u32 compute_dda_inc(unsigned int in, unsigned int out, bool v, unsigned int bpp) { fixed20_12 outf = dfixed_init(out); + fixed20_12 inf = dfixed_init(in); u32 dda_inc; int max;
@@ -79,9 +178,10 @@ static inline u32 compute_dda_inc(fixed20_12 inf, unsigned int out, bool v, return dda_inc; }
-static inline u32 compute_initial_dda(fixed20_12 in) +static inline u32 compute_initial_dda(unsigned int in) { - return dfixed_frac(in); + fixed20_12 inf = dfixed_init(in); + return dfixed_frac(inf); }
static int tegra_dc_set_timings(struct tegra_dc *dc, @@ -152,6 +252,111 @@ static int tegra_crtc_setup_clk(struct drm_crtc *crtc, return 0; }
+int tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index, + const struct tegra_dc_window *window) +{ + unsigned h_offset, v_offset, h_size, v_size, h_dda, v_dda, bpp; + unsigned long value; + + bpp = window->bits_per_pixel / 8; + + value = WINDOW_A_SELECT << index; + tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER); + + tegra_dc_writel(dc, window->format, DC_WIN_COLOR_DEPTH); + tegra_dc_writel(dc, 0, DC_WIN_BYTE_SWAP); + + value = V_POSITION(window->dst.y) | H_POSITION(window->dst.x); + tegra_dc_writel(dc, value, DC_WIN_POSITION); + + value = V_SIZE(window->dst.h) | H_SIZE(window->dst.w); + tegra_dc_writel(dc, value, DC_WIN_SIZE); + + h_offset = window->src.x * bpp; + v_offset = window->src.y; + h_size = window->src.w * bpp; + v_size = window->src.h; + + value = V_PRESCALED_SIZE(v_size) | H_PRESCALED_SIZE(h_size); + tegra_dc_writel(dc, value, DC_WIN_PRESCALED_SIZE); + + h_dda = compute_dda_inc(window->src.w, window->dst.w, false, bpp); + v_dda = compute_dda_inc(window->src.h, window->dst.h, true, bpp); + + value = V_DDA_INC(v_dda) | H_DDA_INC(h_dda); + tegra_dc_writel(dc, value, DC_WIN_DDA_INC); + + h_dda = compute_initial_dda(window->src.x); + v_dda = compute_initial_dda(window->src.y); + + tegra_dc_writel(dc, h_dda, DC_WIN_H_INITIAL_DDA); + tegra_dc_writel(dc, v_dda, DC_WIN_V_INITIAL_DDA); + + tegra_dc_writel(dc, 0, DC_WIN_UV_BUF_STRIDE); + tegra_dc_writel(dc, 0, DC_WIN_BUF_STRIDE); + + tegra_dc_writel(dc, window->base, DC_WINBUF_START_ADDR); + tegra_dc_writel(dc, window->stride, DC_WIN_LINE_STRIDE); + tegra_dc_writel(dc, h_offset, DC_WINBUF_ADDR_H_OFFSET); + tegra_dc_writel(dc, v_offset, DC_WINBUF_ADDR_V_OFFSET); + + value = WIN_ENABLE; + + if (bpp < 24) + value |= COLOR_EXPAND; + + tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS); + + /* + * Disable blending and assume Window A is the bottom-most window, + * Window C is the top-most window and Window B is in the middle. + */ + tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_NOKEY); + tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_1WIN); + + switch (index) { + case 0: + tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_2WIN_X); + tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_2WIN_Y); + tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_3WIN_XY); + break; + + case 1: + tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_2WIN_X); + tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_2WIN_Y); + tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_3WIN_XY); + break; + + case 2: + tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_2WIN_X); + tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_2WIN_Y); + tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_3WIN_XY); + break; + } + + tegra_dc_writel(dc, WIN_A_UPDATE << index, DC_CMD_STATE_CONTROL); + tegra_dc_writel(dc, WIN_A_ACT_REQ << index, DC_CMD_STATE_CONTROL); + + return 0; +} + +unsigned int tegra_dc_format(uint32_t format) +{ + switch (format) { + case DRM_FORMAT_XRGB8888: + return WIN_COLOR_DEPTH_B8G8R8A8; + + case DRM_FORMAT_RGB565: + return WIN_COLOR_DEPTH_B5G6R5; + + default: + break; + } + + WARN(1, "unsupported pixel format %u, using default\n", format); + return WIN_COLOR_DEPTH_B8G8R8A8; +} + static int tegra_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted, @@ -159,8 +364,7 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc, { struct tegra_framebuffer *fb = to_tegra_fb(crtc->fb); struct tegra_dc *dc = to_tegra_dc(crtc); - unsigned int h_dda, v_dda, bpp; - struct tegra_dc_window win; + struct tegra_dc_window window; unsigned long div, value; int err;
@@ -191,81 +395,23 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc, tegra_dc_writel(dc, value, DC_DISP_DISP_CLOCK_CONTROL);
/* setup window parameters */ - memset(&win, 0, sizeof(win)); - win.x.full = dfixed_const(0); - win.y.full = dfixed_const(0); - win.w.full = dfixed_const(mode->hdisplay); - win.h.full = dfixed_const(mode->vdisplay); - win.outx = 0; - win.outy = 0; - win.outw = mode->hdisplay; - win.outh = mode->vdisplay; - - switch (crtc->fb->pixel_format) { - case DRM_FORMAT_XRGB8888: - win.fmt = WIN_COLOR_DEPTH_B8G8R8A8; - break; - - case DRM_FORMAT_RGB565: - win.fmt = WIN_COLOR_DEPTH_B5G6R5; - break; - - default: - win.fmt = WIN_COLOR_DEPTH_B8G8R8A8; - WARN_ON(1); - break; - } - - bpp = crtc->fb->bits_per_pixel / 8; - win.stride = crtc->fb->pitches[0]; - - /* program window registers */ - value = WINDOW_A_SELECT; - tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER); - - tegra_dc_writel(dc, win.fmt, DC_WIN_COLOR_DEPTH); - tegra_dc_writel(dc, 0, DC_WIN_BYTE_SWAP); - - value = V_POSITION(win.outy) | H_POSITION(win.outx); - tegra_dc_writel(dc, value, DC_WIN_POSITION); - - value = V_SIZE(win.outh) | H_SIZE(win.outw); - tegra_dc_writel(dc, value, DC_WIN_SIZE); - - value = V_PRESCALED_SIZE(dfixed_trunc(win.h)) | - H_PRESCALED_SIZE(dfixed_trunc(win.w) * bpp); - tegra_dc_writel(dc, value, DC_WIN_PRESCALED_SIZE); - - h_dda = compute_dda_inc(win.w, win.outw, false, bpp); - v_dda = compute_dda_inc(win.h, win.outh, true, bpp); - - value = V_DDA_INC(v_dda) | H_DDA_INC(h_dda); - tegra_dc_writel(dc, value, DC_WIN_DDA_INC); - - h_dda = compute_initial_dda(win.x); - v_dda = compute_initial_dda(win.y); - - tegra_dc_writel(dc, h_dda, DC_WIN_H_INITIAL_DDA); - tegra_dc_writel(dc, v_dda, DC_WIN_V_INITIAL_DDA); - - tegra_dc_writel(dc, 0, DC_WIN_UV_BUF_STRIDE); - tegra_dc_writel(dc, 0, DC_WIN_BUF_STRIDE); - - tegra_dc_writel(dc, fb->obj->paddr, DC_WINBUF_START_ADDR); - tegra_dc_writel(dc, win.stride, DC_WIN_LINE_STRIDE); - tegra_dc_writel(dc, dfixed_trunc(win.x) * bpp, - DC_WINBUF_ADDR_H_OFFSET); - tegra_dc_writel(dc, dfixed_trunc(win.y), DC_WINBUF_ADDR_V_OFFSET); - - value = WIN_ENABLE; - - if (bpp < 24) - value |= COLOR_EXPAND; - - tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS); - - tegra_dc_writel(dc, 0xff00, DC_WIN_BLEND_NOKEY); - tegra_dc_writel(dc, 0xff00, DC_WIN_BLEND_1WIN); + memset(&window, 0, sizeof(window)); + window.src.x = 0; + window.src.y = 0; + window.src.w = mode->hdisplay; + window.src.h = mode->vdisplay; + window.dst.x = 0; + window.dst.y = 0; + window.dst.w = mode->hdisplay; + window.dst.h = mode->vdisplay; + window.format = tegra_dc_format(crtc->fb->pixel_format); + window.bits_per_pixel = crtc->fb->bits_per_pixel; + window.stride = crtc->fb->pitches[0]; + window.base = fb->obj->paddr; + + err = tegra_dc_setup_window(dc, 0, &window); + if (err < 0) + dev_err(dc->dev, "failed to enable root plane\n");
return 0; } @@ -346,7 +492,7 @@ static void tegra_crtc_load_lut(struct drm_crtc *crtc) }
static const struct drm_crtc_helper_funcs tegra_crtc_helper_funcs = { - .dpms = tegra_crtc_dpms, + .disable = tegra_crtc_disable, .mode_fixup = tegra_crtc_mode_fixup, .mode_set = tegra_crtc_mode_set, .prepare = tegra_crtc_prepare, @@ -587,7 +733,7 @@ static int tegra_dc_show_regs(struct seq_file *s, void *data) DUMP_REG(DC_WIN_BLEND_1WIN); DUMP_REG(DC_WIN_BLEND_2WIN_X); DUMP_REG(DC_WIN_BLEND_2WIN_Y); - DUMP_REG(DC_WIN_BLEND32WIN_XY); + DUMP_REG(DC_WIN_BLEND_3WIN_XY); DUMP_REG(DC_WIN_HP_FETCH_CONTROL); DUMP_REG(DC_WINBUF_START_ADDR); DUMP_REG(DC_WINBUF_START_ADDR_NS); @@ -689,6 +835,10 @@ static int tegra_dc_drm_init(struct host1x_client *client, return err; }
+ err = tegra_dc_add_planes(drm, dc); + if (err < 0) + return err; + if (IS_ENABLED(CONFIG_DEBUG_FS)) { err = tegra_dc_debugfs_init(dc, drm->primary); if (err < 0) diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h index 99977b5..ccfc220 100644 --- a/drivers/gpu/drm/tegra/dc.h +++ b/drivers/gpu/drm/tegra/dc.h @@ -359,7 +359,7 @@ #define DC_WIN_BLEND_1WIN 0x710 #define DC_WIN_BLEND_2WIN_X 0x711 #define DC_WIN_BLEND_2WIN_Y 0x712 -#define DC_WIN_BLEND32WIN_XY 0x713 +#define DC_WIN_BLEND_3WIN_XY 0x713
#define DC_WIN_HP_FETCH_CONTROL 0x714
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h index c7a0507..8202e38 100644 --- a/drivers/gpu/drm/tegra/drm.h +++ b/drivers/gpu/drm/tegra/drm.h @@ -31,6 +31,11 @@ static inline struct tegra_framebuffer *to_tegra_fb(struct drm_framebuffer *fb) return container_of(fb, struct tegra_framebuffer, base); }
+static inline unsigned long tegra_framebuffer_base(struct drm_framebuffer *fb) +{ + return to_tegra_fb(fb)->obj->paddr; +} + struct host1x { struct drm_device *drm; struct device *dev; @@ -121,6 +126,30 @@ static inline unsigned long tegra_dc_readl(struct tegra_dc *dc, return readl(dc->regs + (reg << 2)); }
+struct tegra_dc_window { + struct { + unsigned int x; + unsigned int y; + unsigned int w; + unsigned int h; + } src; + struct { + unsigned int x; + unsigned int y; + unsigned int w; + unsigned int h; + } dst; + unsigned int bits_per_pixel; + unsigned int format; + unsigned int stride; + unsigned long base; +}; + +/* from dc.c */ +extern unsigned int tegra_dc_format(uint32_t format); +extern int tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index, + const struct tegra_dc_window *window); + struct display { unsigned int width; unsigned int height;
On 02/14/2013 12:05 AM, Thierry Reding wrote:
Add support for the B and C planes which support RGB and YUV pixel formats and can be used as overlays or hardware cursor. Currently only 32-bit RGBA pixel formats are advertised.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de
[...]
+static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc) +{
- unsigned int i;
- int err = 0;
- for (i = 0; i < 2; i++) {
struct tegra_plane *plane;plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL);
Using "devm_kzalloc" here seems like not a good idea. Everytime plane disable or crtc disable, we should free "struct tegra_plane" which is allocated here. But the memory which devm_kzalloc allocates is only freed when the driver detach. This makes lots of memory can't be recycled when the plane enable/disable frequently.
if (!plane)return -ENOMEM;plane->index = 1 + i;err = drm_plane_init(drm, &plane->base, 1 << dc->pipe,&tegra_plane_funcs, plane_formats,ARRAY_SIZE(plane_formats), false);if (err < 0)return err;- }
- return 0;
+}
static const struct drm_crtc_funcs tegra_crtc_funcs = { .set_config = drm_crtc_helper_set_config, .destroy = drm_crtc_cleanup, };
-static void tegra_crtc_dpms(struct drm_crtc *crtc, int mode) +static void tegra_crtc_disable(struct drm_crtc *crtc) {
- struct drm_device *drm = crtc->dev;
- struct drm_plane *plane;
- list_for_each_entry(plane, &drm->mode_config.plane_list, head) {
if (plane->crtc == crtc) {tegra_plane_disable(plane);plane->crtc = NULL;if (plane->fb) {drm_framebuffer_unreference(plane->fb);plane->fb = NULL;}}- }
If what I mentioned above(about using devm_kzalloc to allocate "struct tegra_plane") is correct, we need to free "struct tegra_plane" here.
}
static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc, @@ -46,10 +144,11 @@ static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc, return true; }
[...]
On Mon, Feb 18, 2013 at 02:06:56PM +0800, Mark Zhang wrote:
On 02/14/2013 12:05 AM, Thierry Reding wrote:
Add support for the B and C planes which support RGB and YUV pixel formats and can be used as overlays or hardware cursor. Currently only 32-bit RGBA pixel formats are advertised.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de
[...]
+static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc) +{
- unsigned int i;
- int err = 0;
- for (i = 0; i < 2; i++) {
struct tegra_plane *plane;plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL);Using "devm_kzalloc" here seems like not a good idea. Everytime plane disable or crtc disable, we should free "struct tegra_plane" which is allocated here. But the memory which devm_kzalloc allocates is only freed when the driver detach. This makes lots of memory can't be recycled when the plane enable/disable frequently.
Why would we want to do that? I don't think doing so would even work, since the planes are resources that need to be registered with the DRM core and therefore can't be allocated on-demand.
Thierry
On 02/18/2013 02:40 PM, Thierry Reding wrote:
On Mon, Feb 18, 2013 at 02:06:56PM +0800, Mark Zhang wrote:
On 02/14/2013 12:05 AM, Thierry Reding wrote:
Add support for the B and C planes which support RGB and YUV pixel formats and can be used as overlays or hardware cursor. Currently only 32-bit RGBA pixel formats are advertised.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de
[...]
+static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc) +{
- unsigned int i;
- int err = 0;
- for (i = 0; i < 2; i++) {
struct tegra_plane *plane;plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL);Using "devm_kzalloc" here seems like not a good idea. Everytime plane disable or crtc disable, we should free "struct tegra_plane" which is allocated here. But the memory which devm_kzalloc allocates is only freed when the driver detach. This makes lots of memory can't be recycled when the plane enable/disable frequently.
Why would we want to do that? I don't think doing so would even work, since the planes are resources that need to be registered with the DRM core and therefore can't be allocated on-demand.
I'm wondering if we add power management codes in the future, the crtc/plane will be disabled/enabled frequently, e.g: system going to suspend state then resume.
Thierry
On Mon, Feb 18, 2013 at 02:55:04PM +0800, Mark Zhang wrote:
On 02/18/2013 02:40 PM, Thierry Reding wrote:
On Mon, Feb 18, 2013 at 02:06:56PM +0800, Mark Zhang wrote:
On 02/14/2013 12:05 AM, Thierry Reding wrote:
Add support for the B and C planes which support RGB and YUV pixel formats and can be used as overlays or hardware cursor. Currently only 32-bit RGBA pixel formats are advertised.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de
[...]
+static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc) +{
- unsigned int i;
- int err = 0;
- for (i = 0; i < 2; i++) {
struct tegra_plane *plane;plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL);Using "devm_kzalloc" here seems like not a good idea. Everytime plane disable or crtc disable, we should free "struct tegra_plane" which is allocated here. But the memory which devm_kzalloc allocates is only freed when the driver detach. This makes lots of memory can't be recycled when the plane enable/disable frequently.
Why would we want to do that? I don't think doing so would even work, since the planes are resources that need to be registered with the DRM core and therefore can't be allocated on-demand.
I'm wondering if we add power management codes in the future, the crtc/plane will be disabled/enabled frequently, e.g: system going to suspend state then resume.
In that case it makes even less sense to allocate and deallocate the plane every time around. However, any optimizations aside, the allocated memory is passed into the core via drm_plane_init(), which registers that plane with the given CRTC. So if we deallocate the memory when the plane when it is disabled, we'd have to make sure to remove it from the core as well. However that would also mean that it disappears from the list of planes supported by the CRTC and therefore we wouldn't be able to use it again.
Thierry
On 02/18/2013 03:01 PM, Thierry Reding wrote:
On Mon, Feb 18, 2013 at 02:55:04PM +0800, Mark Zhang wrote:
On 02/18/2013 02:40 PM, Thierry Reding wrote:
On Mon, Feb 18, 2013 at 02:06:56PM +0800, Mark Zhang wrote:
On 02/14/2013 12:05 AM, Thierry Reding wrote:
Add support for the B and C planes which support RGB and YUV pixel formats and can be used as overlays or hardware cursor. Currently only 32-bit RGBA pixel formats are advertised.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de
[...]
+static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc) +{
- unsigned int i;
- int err = 0;
- for (i = 0; i < 2; i++) {
struct tegra_plane *plane;plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL);Using "devm_kzalloc" here seems like not a good idea. Everytime plane disable or crtc disable, we should free "struct tegra_plane" which is allocated here. But the memory which devm_kzalloc allocates is only freed when the driver detach. This makes lots of memory can't be recycled when the plane enable/disable frequently.
Why would we want to do that? I don't think doing so would even work, since the planes are resources that need to be registered with the DRM core and therefore can't be allocated on-demand.
I'm wondering if we add power management codes in the future, the crtc/plane will be disabled/enabled frequently, e.g: system going to suspend state then resume.
In that case it makes even less sense to allocate and deallocate the plane every time around. However, any optimizations aside, the allocated memory is passed into the core via drm_plane_init(), which registers that plane with the given CRTC. So if we deallocate the memory when the plane when it is disabled, we'd have to make sure to remove it from the core as well. However that would also mean that it disappears from the list of planes supported by the CRTC and therefore we wouldn't be able to use it again.
Alright, I mixed up the "disable" & "remove" of planes and crtcs. You're right. Just forget what I said in this thread.
Mark
Thierry
The sequence for replacing the scanout buffer is much shorter than a full mode change operation so implementing this callback considerably speeds up cases where only a new framebuffer is to be scanned out.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de --- Changes in v3: - split DC_CMD_STATE_CONTROL writes
drivers/gpu/drm/tegra/dc.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 8f97b1c..cc4c85e 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -114,6 +114,27 @@ static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc) return 0; }
+static int tegra_dc_set_base(struct tegra_dc *dc, int x, int y, + struct tegra_framebuffer *fb) +{ + unsigned long value; + + tegra_dc_writel(dc, WINDOW_A_SELECT, DC_CMD_DISPLAY_WINDOW_HEADER); + + value = fb->base.offsets[0] + y * fb->base.pitches[0] + + x * fb->base.bits_per_pixel / 8; + + tegra_dc_writel(dc, fb->obj->paddr + value, DC_WINBUF_START_ADDR); + + value = GENERAL_UPDATE | WIN_A_UPDATE; + tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL); + + value = GENERAL_ACT_REQ | WIN_A_ACT_REQ; + tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL); + + return 0; +} + static const struct drm_crtc_funcs tegra_crtc_funcs = { .set_config = drm_crtc_helper_set_config, .destroy = drm_crtc_cleanup, @@ -416,6 +437,15 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc, return 0; }
+static int tegra_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, + struct drm_framebuffer *old_fb) +{ + struct tegra_framebuffer *fb = to_tegra_fb(crtc->fb); + struct tegra_dc *dc = to_tegra_dc(crtc); + + return tegra_dc_set_base(dc, x, y, fb); +} + static void tegra_crtc_prepare(struct drm_crtc *crtc) { struct tegra_dc *dc = to_tegra_dc(crtc); @@ -495,6 +525,7 @@ static const struct drm_crtc_helper_funcs tegra_crtc_helper_funcs = { .disable = tegra_crtc_disable, .mode_fixup = tegra_crtc_mode_fixup, .mode_set = tegra_crtc_mode_set, + .mode_set_base = tegra_crtc_mode_set_base, .prepare = tegra_crtc_prepare, .commit = tegra_crtc_commit, .load_lut = tegra_crtc_load_lut,
On 02/14/2013 12:05 AM, Thierry Reding wrote:
The sequence for replacing the scanout buffer is much shorter than a full mode change operation so implementing this callback considerably speeds up cases where only a new framebuffer is to be scanned out.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de
Changes in v3:
- split DC_CMD_STATE_CONTROL writes
drivers/gpu/drm/tegra/dc.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 8f97b1c..cc4c85e 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -114,6 +114,27 @@ static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc) return 0; }
+static int tegra_dc_set_base(struct tegra_dc *dc, int x, int y,
struct tegra_framebuffer *fb)+{
- unsigned long value;
- tegra_dc_writel(dc, WINDOW_A_SELECT, DC_CMD_DISPLAY_WINDOW_HEADER);
- value = fb->base.offsets[0] + y * fb->base.pitches[0] +
x * fb->base.bits_per_pixel / 8;- tegra_dc_writel(dc, fb->obj->paddr + value, DC_WINBUF_START_ADDR);
- value = GENERAL_UPDATE | WIN_A_UPDATE;
- tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
- tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- return 0;
+}
Again, what do you think about the "line stride" problem I mentioned:
http://lists.freedesktop.org/archives/dri-devel/2013-January/033561.html
Don't get me wrong that I also don't want to add a line stride update here because that doesn't make sense. It's just a workaround. But we need to find a way to make multi-head page flip working.
Mark
static const struct drm_crtc_funcs tegra_crtc_funcs = { .set_config = drm_crtc_helper_set_config, .destroy = drm_crtc_cleanup, @@ -416,6 +437,15 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc, return 0; }
+static int tegra_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *old_fb)+{
- struct tegra_framebuffer *fb = to_tegra_fb(crtc->fb);
- struct tegra_dc *dc = to_tegra_dc(crtc);
- return tegra_dc_set_base(dc, x, y, fb);
+}
static void tegra_crtc_prepare(struct drm_crtc *crtc) { struct tegra_dc *dc = to_tegra_dc(crtc); @@ -495,6 +525,7 @@ static const struct drm_crtc_helper_funcs tegra_crtc_helper_funcs = { .disable = tegra_crtc_disable, .mode_fixup = tegra_crtc_mode_fixup, .mode_set = tegra_crtc_mode_set,
- .mode_set_base = tegra_crtc_mode_set_base, .prepare = tegra_crtc_prepare, .commit = tegra_crtc_commit, .load_lut = tegra_crtc_load_lut,
On 02/18/2013 02:17 PM, Mark Zhang wrote:
On 02/14/2013 12:05 AM, Thierry Reding wrote:
The sequence for replacing the scanout buffer is much shorter than a full mode change operation so implementing this callback considerably speeds up cases where only a new framebuffer is to be scanned out.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de
Changes in v3:
- split DC_CMD_STATE_CONTROL writes
drivers/gpu/drm/tegra/dc.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 8f97b1c..cc4c85e 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -114,6 +114,27 @@ static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc) return 0; }
+static int tegra_dc_set_base(struct tegra_dc *dc, int x, int y,
struct tegra_framebuffer *fb)+{
- unsigned long value;
- tegra_dc_writel(dc, WINDOW_A_SELECT, DC_CMD_DISPLAY_WINDOW_HEADER);
- value = fb->base.offsets[0] + y * fb->base.pitches[0] +
x * fb->base.bits_per_pixel / 8;- tegra_dc_writel(dc, fb->obj->paddr + value, DC_WINBUF_START_ADDR);
- value = GENERAL_UPDATE | WIN_A_UPDATE;
- tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
- tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- return 0;
+}
Again, what do you think about the "line stride" problem I mentioned:
http://lists.freedesktop.org/archives/dri-devel/2013-January/033561.html
Don't get me wrong that I also don't want to add a line stride update here because that doesn't make sense. It's just a workaround. But we need to find a way to make multi-head page flip working.
Sorry, it's not "multi-head page flip", it should be "multi-head fb change". For example, if LVDS & HDMI are connected, I can create 4 fbs for them(every is double-buffered) and call drmModeSetCrtc to switch 2 fbs of LVDS & HDMI to show something.
Mark
Mark
static const struct drm_crtc_funcs tegra_crtc_funcs = { .set_config = drm_crtc_helper_set_config, .destroy = drm_crtc_cleanup, @@ -416,6 +437,15 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc, return 0; }
+static int tegra_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *old_fb)+{
- struct tegra_framebuffer *fb = to_tegra_fb(crtc->fb);
- struct tegra_dc *dc = to_tegra_dc(crtc);
- return tegra_dc_set_base(dc, x, y, fb);
+}
static void tegra_crtc_prepare(struct drm_crtc *crtc) { struct tegra_dc *dc = to_tegra_dc(crtc); @@ -495,6 +525,7 @@ static const struct drm_crtc_helper_funcs tegra_crtc_helper_funcs = { .disable = tegra_crtc_disable, .mode_fixup = tegra_crtc_mode_fixup, .mode_set = tegra_crtc_mode_set,
- .mode_set_base = tegra_crtc_mode_set_base, .prepare = tegra_crtc_prepare, .commit = tegra_crtc_commit, .load_lut = tegra_crtc_load_lut,
On Mon, Feb 18, 2013 at 02:17:53PM +0800, Mark Zhang wrote:
On 02/14/2013 12:05 AM, Thierry Reding wrote:
The sequence for replacing the scanout buffer is much shorter than a full mode change operation so implementing this callback considerably speeds up cases where only a new framebuffer is to be scanned out.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de
Changes in v3:
- split DC_CMD_STATE_CONTROL writes
drivers/gpu/drm/tegra/dc.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 8f97b1c..cc4c85e 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -114,6 +114,27 @@ static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc) return 0; }
+static int tegra_dc_set_base(struct tegra_dc *dc, int x, int y,
struct tegra_framebuffer *fb)+{
- unsigned long value;
- tegra_dc_writel(dc, WINDOW_A_SELECT, DC_CMD_DISPLAY_WINDOW_HEADER);
- value = fb->base.offsets[0] + y * fb->base.pitches[0] +
x * fb->base.bits_per_pixel / 8;- tegra_dc_writel(dc, fb->obj->paddr + value, DC_WINBUF_START_ADDR);
- value = GENERAL_UPDATE | WIN_A_UPDATE;
- tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
- tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- return 0;
+}
Again, what do you think about the "line stride" problem I mentioned:
http://lists.freedesktop.org/archives/dri-devel/2013-January/033561.html
Don't get me wrong that I also don't want to add a line stride update here because that doesn't make sense. It's just a workaround. But we need to find a way to make multi-head page flip working.
I'm not sure that it's something we need to support. .mode_set_base() is explicitly used only for cases where the framebuffer configuration doesn't change. That is, only in cases where the only thing that changes is the physical address of the framebuffer to be displayed.
The current case where one framebuffer is used as scanout for both outputs isn't something that page-flipping can support. Page-flipping is always per-CRTC because typically each CRTC would run at a different frequency (or even if both ran at the same frequency the VBLANK is very unlikely to coincide anyway).
So an application that wants to support page-flipping on two outputs also needs to take care to set them up with different framebuffers, in which case what you're describing here can't really happen.
Thierry
On 02/18/2013 02:48 PM, Thierry Reding wrote:
On Mon, Feb 18, 2013 at 02:17:53PM +0800, Mark Zhang wrote:
On 02/14/2013 12:05 AM, Thierry Reding wrote:
The sequence for replacing the scanout buffer is much shorter than a full mode change operation so implementing this callback considerably speeds up cases where only a new framebuffer is to be scanned out.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de
Changes in v3:
- split DC_CMD_STATE_CONTROL writes
drivers/gpu/drm/tegra/dc.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 8f97b1c..cc4c85e 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -114,6 +114,27 @@ static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc) return 0; }
+static int tegra_dc_set_base(struct tegra_dc *dc, int x, int y,
struct tegra_framebuffer *fb)+{
- unsigned long value;
- tegra_dc_writel(dc, WINDOW_A_SELECT, DC_CMD_DISPLAY_WINDOW_HEADER);
- value = fb->base.offsets[0] + y * fb->base.pitches[0] +
x * fb->base.bits_per_pixel / 8;- tegra_dc_writel(dc, fb->obj->paddr + value, DC_WINBUF_START_ADDR);
- value = GENERAL_UPDATE | WIN_A_UPDATE;
- tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
- tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- return 0;
+}
Again, what do you think about the "line stride" problem I mentioned:
http://lists.freedesktop.org/archives/dri-devel/2013-January/033561.html
Don't get me wrong that I also don't want to add a line stride update here because that doesn't make sense. It's just a workaround. But we need to find a way to make multi-head page flip working.
I'm not sure that it's something we need to support. .mode_set_base() is explicitly used only for cases where the framebuffer configuration doesn't change. That is, only in cases where the only thing that changes is the physical address of the framebuffer to be displayed.
The current case where one framebuffer is used as scanout for both outputs isn't something that page-flipping can support. Page-flipping is always per-CRTC because typically each CRTC would run at a different frequency (or even if both ran at the same frequency the VBLANK is very unlikely to coincide anyway).
So an application that wants to support page-flipping on two outputs also needs to take care to set them up with different framebuffers, in which case what you're describing here can't really happen.
Yes, the userspace application needs to setup different framebuffers for different crtcs. So if LVDS & HDMI are connected, here is what the application does:
1. drm reports that 2 connectors: LVDS & HDMI are present in the system 2. The resolution of them are: 1366x768 & 1080p 3. Userspace application allocates 2 fbs according to the resolution got above. 4. call drmModeSetCrtc to switch the fb shown in LVDS to the new fb we created. 5. At this time, remember the line stride setting in dc which connects with LVDS is calculated according to the 1080p resolution. So finally we got a corrupt display because we're showing a 1366x768 fb but with 1080p's line stride.
Hope I explained this clear. Try this test application if you still have problems:
https://github.com/dvdhrm/docs/blob/master/drm-howto/modeset-vsync.c
Mark
Thierry
On Mon, Feb 18, 2013 at 03:06:10PM +0800, Mark Zhang wrote:
On 02/18/2013 02:48 PM, Thierry Reding wrote:
On Mon, Feb 18, 2013 at 02:17:53PM +0800, Mark Zhang wrote:
On 02/14/2013 12:05 AM, Thierry Reding wrote:
The sequence for replacing the scanout buffer is much shorter than a full mode change operation so implementing this callback considerably speeds up cases where only a new framebuffer is to be scanned out.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de
Changes in v3:
- split DC_CMD_STATE_CONTROL writes
drivers/gpu/drm/tegra/dc.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 8f97b1c..cc4c85e 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -114,6 +114,27 @@ static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc) return 0; }
+static int tegra_dc_set_base(struct tegra_dc *dc, int x, int y,
struct tegra_framebuffer *fb)+{
- unsigned long value;
- tegra_dc_writel(dc, WINDOW_A_SELECT, DC_CMD_DISPLAY_WINDOW_HEADER);
- value = fb->base.offsets[0] + y * fb->base.pitches[0] +
x * fb->base.bits_per_pixel / 8;- tegra_dc_writel(dc, fb->obj->paddr + value, DC_WINBUF_START_ADDR);
- value = GENERAL_UPDATE | WIN_A_UPDATE;
- tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
- tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- return 0;
+}
Again, what do you think about the "line stride" problem I mentioned:
http://lists.freedesktop.org/archives/dri-devel/2013-January/033561.html
Don't get me wrong that I also don't want to add a line stride update here because that doesn't make sense. It's just a workaround. But we need to find a way to make multi-head page flip working.
I'm not sure that it's something we need to support. .mode_set_base() is explicitly used only for cases where the framebuffer configuration doesn't change. That is, only in cases where the only thing that changes is the physical address of the framebuffer to be displayed.
The current case where one framebuffer is used as scanout for both outputs isn't something that page-flipping can support. Page-flipping is always per-CRTC because typically each CRTC would run at a different frequency (or even if both ran at the same frequency the VBLANK is very unlikely to coincide anyway).
So an application that wants to support page-flipping on two outputs also needs to take care to set them up with different framebuffers, in which case what you're describing here can't really happen.
Yes, the userspace application needs to setup different framebuffers for different crtcs. So if LVDS & HDMI are connected, here is what the application does:
- drm reports that 2 connectors: LVDS & HDMI are present in the system
- The resolution of them are: 1366x768 & 1080p
- Userspace application allocates 2 fbs according to the resolution got
above. 4. call drmModeSetCrtc to switch the fb shown in LVDS to the new fb we created. 5. At this time, remember the line stride setting in dc which connects with LVDS is calculated according to the 1080p resolution. So finally we got a corrupt display because we're showing a 1366x768 fb but with 1080p's line stride.
I don't see how the 1080p stride would still be relevant here. Setting the LVDS to the new 1366x768 framebuffer should trigger a full modeset and therefore set the stride to the value required by the new 1366x768 framebuffer.
Thierry
On 02/18/2013 03:20 PM, Thierry Reding wrote:
On Mon, Feb 18, 2013 at 03:06:10PM +0800, Mark Zhang wrote:
On 02/18/2013 02:48 PM, Thierry Reding wrote:
On Mon, Feb 18, 2013 at 02:17:53PM +0800, Mark Zhang wrote:
On 02/14/2013 12:05 AM, Thierry Reding wrote:
The sequence for replacing the scanout buffer is much shorter than a full mode change operation so implementing this callback considerably speeds up cases where only a new framebuffer is to be scanned out.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de
Changes in v3:
- split DC_CMD_STATE_CONTROL writes
drivers/gpu/drm/tegra/dc.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 8f97b1c..cc4c85e 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -114,6 +114,27 @@ static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc) return 0; }
+static int tegra_dc_set_base(struct tegra_dc *dc, int x, int y,
struct tegra_framebuffer *fb)+{
- unsigned long value;
- tegra_dc_writel(dc, WINDOW_A_SELECT, DC_CMD_DISPLAY_WINDOW_HEADER);
- value = fb->base.offsets[0] + y * fb->base.pitches[0] +
x * fb->base.bits_per_pixel / 8;- tegra_dc_writel(dc, fb->obj->paddr + value, DC_WINBUF_START_ADDR);
- value = GENERAL_UPDATE | WIN_A_UPDATE;
- tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
- tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- return 0;
+}
Again, what do you think about the "line stride" problem I mentioned:
http://lists.freedesktop.org/archives/dri-devel/2013-January/033561.html
Don't get me wrong that I also don't want to add a line stride update here because that doesn't make sense. It's just a workaround. But we need to find a way to make multi-head page flip working.
I'm not sure that it's something we need to support. .mode_set_base() is explicitly used only for cases where the framebuffer configuration doesn't change. That is, only in cases where the only thing that changes is the physical address of the framebuffer to be displayed.
The current case where one framebuffer is used as scanout for both outputs isn't something that page-flipping can support. Page-flipping is always per-CRTC because typically each CRTC would run at a different frequency (or even if both ran at the same frequency the VBLANK is very unlikely to coincide anyway).
So an application that wants to support page-flipping on two outputs also needs to take care to set them up with different framebuffers, in which case what you're describing here can't really happen.
Yes, the userspace application needs to setup different framebuffers for different crtcs. So if LVDS & HDMI are connected, here is what the application does:
- drm reports that 2 connectors: LVDS & HDMI are present in the system
- The resolution of them are: 1366x768 & 1080p
- Userspace application allocates 2 fbs according to the resolution got
above. 4. call drmModeSetCrtc to switch the fb shown in LVDS to the new fb we created. 5. At this time, remember the line stride setting in dc which connects with LVDS is calculated according to the 1080p resolution. So finally we got a corrupt display because we're showing a 1366x768 fb but with 1080p's line stride.
I don't see how the 1080p stride would still be relevant here. Setting the LVDS to the new 1366x768 framebuffer should trigger a full modeset and therefore set the stride to the value required by the new 1366x768 framebuffer.
Actually the dc which connects with LVDS doesn't know about 1080p stuffs. All the video mode infos stored in this dc is still 1366x768(I just checked the register values on my Tegra 30 cardhu). The dc just scans 1366 pixels then jump to next line(based on the 1080p line stride we set) and keep scanning.
So the drm crtc helper(I believe it's function: drm_crtc_helper_set_config) finds out that video mode is not changed so a fb_change while not full modeset is triggered.
I started to test this new series patch set just now. I'll let you know whether the issue I described still exists.
Mark
Thierry
On 02/18/2013 04:40 PM, Mark Zhang wrote:
On 02/18/2013 03:20 PM, Thierry Reding wrote:
[...]
Actually the dc which connects with LVDS doesn't know about 1080p stuffs. All the video mode infos stored in this dc is still 1366x768(I just checked the register values on my Tegra 30 cardhu). The dc just scans 1366 pixels then jump to next line(based on the 1080p line stride we set) and keep scanning.
So the drm crtc helper(I believe it's function: drm_crtc_helper_set_config) finds out that video mode is not changed so a fb_change while not full modeset is triggered.
I started to test this new series patch set just now. I'll let you know whether the issue I described still exists.
Tested. This issue still exists.
Mark
Mark
Thierry
Implement support for the VBLANK IOCTL. Note that Tegra is somewhat special in this case because it doesn't use the generic IRQ support provided by the DRM core (DRIVER_HAVE_IRQ) but rather registers one interrupt handler for each display controller.
While at it, clean up the way that interrupts are enabled to ensure that the VBLANK interrupt only gets enabled when required.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de --- Changes in v3: - implement dummy .get_vblank_counter()
drivers/gpu/drm/tegra/dc.c | 55 +++++++++++++++++++++++++++++++-------------- drivers/gpu/drm/tegra/drm.c | 50 +++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/tegra/drm.h | 3 +++ 3 files changed, 91 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index cc4c85e..9e21c84 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -135,6 +135,32 @@ static int tegra_dc_set_base(struct tegra_dc *dc, int x, int y, return 0; }
+void tegra_dc_enable_vblank(struct tegra_dc *dc) +{ + unsigned long value, flags; + + spin_lock_irqsave(&dc->lock, flags); + + value = tegra_dc_readl(dc, DC_CMD_INT_MASK); + value |= VBLANK_INT; + tegra_dc_writel(dc, value, DC_CMD_INT_MASK); + + spin_unlock_irqrestore(&dc->lock, flags); +} + +void tegra_dc_disable_vblank(struct tegra_dc *dc) +{ + unsigned long value, flags; + + spin_lock_irqsave(&dc->lock, flags); + + value = tegra_dc_readl(dc, DC_CMD_INT_MASK); + value &= ~VBLANK_INT; + tegra_dc_writel(dc, value, DC_CMD_INT_MASK); + + spin_unlock_irqrestore(&dc->lock, flags); +} + static const struct drm_crtc_funcs tegra_crtc_funcs = { .set_config = drm_crtc_helper_set_config, .destroy = drm_crtc_cleanup, @@ -389,6 +415,8 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc, unsigned long div, value; int err;
+ drm_vblank_pre_modeset(crtc->dev, dc->pipe); + err = tegra_crtc_setup_clk(crtc, mode, &div); if (err) { dev_err(dc->dev, "failed to setup clock for CRTC: %d\n", err); @@ -490,31 +518,23 @@ static void tegra_crtc_prepare(struct drm_crtc *crtc) tegra_dc_writel(dc, value, DC_DISP_DISP_MEM_HIGH_PRIORITY_TIMER);
value = VBLANK_INT | WIN_A_UF_INT | WIN_B_UF_INT | WIN_C_UF_INT; - tegra_dc_writel(dc, value, DC_CMD_INT_MASK); - - value = VBLANK_INT | WIN_A_UF_INT | WIN_B_UF_INT | WIN_C_UF_INT; tegra_dc_writel(dc, value, DC_CMD_INT_ENABLE); + + value = WIN_A_UF_INT | WIN_B_UF_INT | WIN_C_UF_INT; + tegra_dc_writel(dc, value, DC_CMD_INT_MASK); }
static void tegra_crtc_commit(struct drm_crtc *crtc) { struct tegra_dc *dc = to_tegra_dc(crtc); - unsigned long update_mask; unsigned long value;
- update_mask = GENERAL_ACT_REQ | WIN_A_ACT_REQ; - - tegra_dc_writel(dc, update_mask << 8, DC_CMD_STATE_CONTROL); - - value = tegra_dc_readl(dc, DC_CMD_INT_ENABLE); - value |= FRAME_END_INT; - tegra_dc_writel(dc, value, DC_CMD_INT_ENABLE); + value = GENERAL_ACT_REQ | WIN_A_ACT_REQ | + GENERAL_UPDATE | WIN_A_UPDATE;
- value = tegra_dc_readl(dc, DC_CMD_INT_MASK); - value |= FRAME_END_INT; - tegra_dc_writel(dc, value, DC_CMD_INT_MASK); + tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- tegra_dc_writel(dc, update_mask, DC_CMD_STATE_CONTROL); + drm_vblank_post_modeset(crtc->dev, dc->pipe); }
static void tegra_crtc_load_lut(struct drm_crtc *crtc) @@ -531,7 +551,7 @@ static const struct drm_crtc_helper_funcs tegra_crtc_helper_funcs = { .load_lut = tegra_crtc_load_lut, };
-static irqreturn_t tegra_drm_irq(int irq, void *data) +static irqreturn_t tegra_dc_irq(int irq, void *data) { struct tegra_dc *dc = data; unsigned long status; @@ -876,7 +896,7 @@ static int tegra_dc_drm_init(struct host1x_client *client, dev_err(dc->dev, "debugfs setup failed: %d\n", err); }
- err = devm_request_irq(dc->dev, dc->irq, tegra_drm_irq, 0, + err = devm_request_irq(dc->dev, dc->irq, tegra_dc_irq, 0, dev_name(dc->dev), dc); if (err < 0) { dev_err(dc->dev, "failed to request IRQ#%u: %d\n", dc->irq, @@ -925,6 +945,7 @@ static int tegra_dc_probe(struct platform_device *pdev) if (!dc) return -ENOMEM;
+ spin_lock_init(&dc->lock); INIT_LIST_HEAD(&dc->list); dc->dev = &pdev->dev;
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index d980dc7..8a31f44 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -39,6 +39,10 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags) if (err < 0) return err;
+ err = drm_vblank_init(drm, drm->mode_config.num_crtc); + if (err < 0) + return err; + err = tegra_drm_fb_init(drm); if (err < 0) return err; @@ -88,6 +92,48 @@ static const struct file_operations tegra_drm_fops = { .llseek = noop_llseek, };
+static struct drm_crtc *tegra_crtc_from_pipe(struct drm_device *drm, int pipe) +{ + struct drm_crtc *crtc; + + list_for_each_entry(crtc, &drm->mode_config.crtc_list, head) { + struct tegra_dc *dc = to_tegra_dc(crtc); + + if (dc->pipe == pipe) + return crtc; + } + + return NULL; +} + +static u32 tegra_drm_get_vblank_counter(struct drm_device *dev, int crtc) +{ + /* TODO: implement real hardware counter using syncpoints */ + return drm_vblank_count(dev, crtc); +} + +static int tegra_drm_enable_vblank(struct drm_device *drm, int pipe) +{ + struct drm_crtc *crtc = tegra_crtc_from_pipe(drm, pipe); + struct tegra_dc *dc = to_tegra_dc(crtc); + + if (!crtc) + return -ENODEV; + + tegra_dc_enable_vblank(dc); + + return 0; +} + +static void tegra_drm_disable_vblank(struct drm_device *drm, int pipe) +{ + struct drm_crtc *crtc = tegra_crtc_from_pipe(drm, pipe); + struct tegra_dc *dc = to_tegra_dc(crtc); + + if (crtc) + tegra_dc_disable_vblank(dc); +} + struct drm_driver tegra_drm_driver = { .driver_features = DRIVER_BUS_PLATFORM | DRIVER_MODESET | DRIVER_GEM, .load = tegra_drm_load, @@ -95,6 +141,10 @@ struct drm_driver tegra_drm_driver = { .open = tegra_drm_open, .lastclose = tegra_drm_lastclose,
+ .get_vblank_counter = tegra_drm_get_vblank_counter, + .enable_vblank = tegra_drm_enable_vblank, + .disable_vblank = tegra_drm_disable_vblank, + .gem_free_object = drm_gem_cma_free_object, .gem_vm_ops = &drm_gem_cma_vm_ops, .dumb_create = drm_gem_cma_dumb_create, diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h index 8202e38..51bc7b8 100644 --- a/drivers/gpu/drm/tegra/drm.h +++ b/drivers/gpu/drm/tegra/drm.h @@ -83,6 +83,7 @@ struct tegra_output;
struct tegra_dc { struct host1x_client client; + spinlock_t lock;
struct host1x *host1x; struct device *dev; @@ -149,6 +150,8 @@ struct tegra_dc_window { extern unsigned int tegra_dc_format(uint32_t format); extern int tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index, const struct tegra_dc_window *window); +extern void tegra_dc_enable_vblank(struct tegra_dc *dc); +extern void tegra_dc_disable_vblank(struct tegra_dc *dc);
struct display { unsigned int width;
All the necessary support bits like .mode_set_base() and VBLANK are now available, so page-flipping case easily be implemented on top.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de --- Changes in v2: - use drm_send_vblank_event() - set crtc->fb field
drivers/gpu/drm/tegra/dc.c | 66 +++++++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/tegra/drm.c | 9 +++++++ drivers/gpu/drm/tegra/drm.h | 5 ++++ 3 files changed, 80 insertions(+)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 9e21c84..0328963 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -161,7 +161,72 @@ void tegra_dc_disable_vblank(struct tegra_dc *dc) spin_unlock_irqrestore(&dc->lock, flags); }
+static void tegra_dc_finish_page_flip(struct tegra_dc *dc) +{ + struct drm_pending_vblank_event *event; + struct drm_device *drm = dc->base.dev; + unsigned long flags; + + spin_lock_irqsave(&drm->event_lock, flags); + + event = dc->event; + dc->event = NULL; + + if (!event) { + spin_unlock_irqrestore(&drm->event_lock, flags); + return; + } + + drm_send_vblank_event(drm, dc->pipe, event); + drm_vblank_put(drm, dc->pipe); + + spin_unlock_irqrestore(&drm->event_lock, flags); +} + +void tegra_dc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file) +{ + struct tegra_dc *dc = to_tegra_dc(crtc); + struct drm_device *drm = crtc->dev; + unsigned long flags; + + spin_lock_irqsave(&drm->event_lock, flags); + + if (dc->event && dc->event->base.file_priv == file) { + dc->event->base.destroy(&dc->event->base); + drm_vblank_put(drm, dc->pipe); + dc->event = NULL; + } + + spin_unlock_irqrestore(&drm->event_lock, flags); +} + +static int tegra_dc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, + struct drm_pending_vblank_event *event) +{ + struct tegra_framebuffer *newfb = to_tegra_fb(fb); + struct tegra_dc *dc = to_tegra_dc(crtc); + struct drm_device *drm = crtc->dev; + unsigned long flags; + + if (dc->event) + return -EBUSY; + + tegra_dc_set_base(dc, 0, 0, newfb); + crtc->fb = fb; + + if (event) { + event->pipe = dc->pipe; + spin_lock_irqsave(&drm->event_lock, flags); + dc->event = event; + spin_unlock_irqrestore(&drm->event_lock, flags); + drm_vblank_get(drm, dc->pipe); + } + + return 0; +} + static const struct drm_crtc_funcs tegra_crtc_funcs = { + .page_flip = tegra_dc_page_flip, .set_config = drm_crtc_helper_set_config, .destroy = drm_crtc_cleanup, }; @@ -570,6 +635,7 @@ static irqreturn_t tegra_dc_irq(int irq, void *data) dev_dbg(dc->dev, "%s(): vertical blank\n", __func__); */ drm_handle_vblank(dc->base.dev, dc->pipe); + tegra_dc_finish_page_flip(dc); }
if (status & (WIN_A_UF_INT | WIN_B_UF_INT | WIN_C_UF_INT)) { diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index 8a31f44..edaf9c0 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -134,11 +134,20 @@ static void tegra_drm_disable_vblank(struct drm_device *drm, int pipe) tegra_dc_disable_vblank(dc); }
+static void tegra_drm_preclose(struct drm_device *drm, struct drm_file *file) +{ + struct drm_crtc *crtc; + + list_for_each_entry(crtc, &drm->mode_config.crtc_list, head) + tegra_dc_cancel_page_flip(crtc, file); +} + struct drm_driver tegra_drm_driver = { .driver_features = DRIVER_BUS_PLATFORM | DRIVER_MODESET | DRIVER_GEM, .load = tegra_drm_load, .unload = tegra_drm_unload, .open = tegra_drm_open, + .preclose = tegra_drm_preclose, .lastclose = tegra_drm_lastclose,
.get_vblank_counter = tegra_drm_get_vblank_counter, diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h index 51bc7b8..f7d93e4 100644 --- a/drivers/gpu/drm/tegra/drm.h +++ b/drivers/gpu/drm/tegra/drm.h @@ -103,6 +103,9 @@ struct tegra_dc { struct drm_info_list *debugfs_files; struct drm_minor *minor; struct dentry *debugfs; + + /* page-flip handling */ + struct drm_pending_vblank_event *event; };
static inline struct tegra_dc *host1x_client_to_dc(struct host1x_client *client) @@ -152,6 +155,8 @@ extern int tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index, const struct tegra_dc_window *window); extern void tegra_dc_enable_vblank(struct tegra_dc *dc); extern void tegra_dc_disable_vblank(struct tegra_dc *dc); +extern void tegra_dc_cancel_page_flip(struct drm_crtc *crtc, + struct drm_file *file);
struct display { unsigned int width;
The Tegra TRM says that the ACT_REQ and UPDATE fields cannot be programmed at the same time so they are updated in two consecutive writes instead.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de --- drivers/gpu/drm/tegra/dc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 0328963..9b2a551 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -594,9 +594,10 @@ static void tegra_crtc_commit(struct drm_crtc *crtc) struct tegra_dc *dc = to_tegra_dc(crtc); unsigned long value;
- value = GENERAL_ACT_REQ | WIN_A_ACT_REQ | - GENERAL_UPDATE | WIN_A_UPDATE; + value = GENERAL_UPDATE | WIN_A_UPDATE; + tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
+ value = GENERAL_ACT_REQ | WIN_A_ACT_REQ; tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
drm_vblank_post_modeset(crtc->dev, dc->pipe);
This list is most useful to inspect whether framebuffer reference counting works as expected. The code is loosely based on the i915 implementation.
Signed-off-by: Thierry Reding thierry.reding@avionic-design.de --- drivers/gpu/drm/tegra/drm.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index edaf9c0..9d452df 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -142,6 +142,45 @@ static void tegra_drm_preclose(struct drm_device *drm, struct drm_file *file) tegra_dc_cancel_page_flip(crtc, file); }
+#ifdef CONFIG_DEBUG_FS +static int tegra_debugfs_framebuffers(struct seq_file *s, void *data) +{ + struct drm_info_node *node = (struct drm_info_node *)s->private; + struct drm_device *drm = node->minor->dev; + struct drm_framebuffer *fb; + + mutex_lock(&drm->mode_config.fb_lock); + + list_for_each_entry(fb, &drm->mode_config.fb_list, head) { + seq_printf(s, "%3d: user size: %d x %d, depth %d, %d bpp, refcount %d\n", + fb->base.id, fb->width, fb->height, fb->depth, + fb->bits_per_pixel, + atomic_read(&fb->refcount.refcount)); + } + + mutex_unlock(&drm->mode_config.fb_lock); + + return 0; +} + +static struct drm_info_list tegra_debugfs_list[] = { + { "framebuffers", tegra_debugfs_framebuffers, 0 }, +}; + +static int tegra_debugfs_init(struct drm_minor *minor) +{ + return drm_debugfs_create_files(tegra_debugfs_list, + ARRAY_SIZE(tegra_debugfs_list), + minor->debugfs_root, minor); +} + +static void tegra_debugfs_cleanup(struct drm_minor *minor) +{ + drm_debugfs_remove_files(tegra_debugfs_list, + ARRAY_SIZE(tegra_debugfs_list), minor); +} +#endif + struct drm_driver tegra_drm_driver = { .driver_features = DRIVER_BUS_PLATFORM | DRIVER_MODESET | DRIVER_GEM, .load = tegra_drm_load, @@ -154,6 +193,11 @@ struct drm_driver tegra_drm_driver = { .enable_vblank = tegra_drm_enable_vblank, .disable_vblank = tegra_drm_disable_vblank,
+#if defined(CONFIG_DEBUG_FS) + .debugfs_init = tegra_debugfs_init, + .debugfs_cleanup = tegra_debugfs_cleanup, +#endif + .gem_free_object = drm_gem_cma_free_object, .gem_vm_ops = &drm_gem_cma_vm_ops, .dumb_create = drm_gem_cma_dumb_create,
Hi Thierry,
Which version is this patch set based on? I tried to apply it on linux-next 0206 & tot 0215 but failed:
Applying: drm: Add consistency check for page-flipping Applying: drm/tegra: Add plane support error: patch failed: drivers/gpu/drm/tegra/drm.h:121 error: drivers/gpu/drm/tegra/drm.h: patch does not apply Patch failed at 0002 drm/tegra: Add plane support When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort".
Mark On 02/14/2013 12:04 AM, Thierry Reding wrote:
This patch series introduces a number of useful features for the Tegra DRM driver. Patch 1 is a documentation update and adds a warning to the page-flip IOCTL to catch buggy drivers that return success but failed to update the crtc->fb field, which would cause the reference counting to become unbalanced.
Patch 2 adds support for the additional two planes available on Tegra hardware, while patch 3 implement .mode_set_base() to allow for some nice speed up when changing the framebuffer without actually changing the resolution. Patch 4 adds VBLANK support and patch 5 builds on the previous two to provide the page-flipping IOCTL. Patch 6 splits the DC_CMD_STATE_CONTROL register writes into two consecutive writes as required by the TRM.
Finally patch 7 adds a new file, named "framebuffers" to debugfs which can be used to dump a list of framebuffers attached to the DRM device. This is most useful to inspect the reference count but could also be helpful in diagnosing out-of-memory conditions and such.
Thierry
Thierry Reding (7): drm: Add consistency check for page-flipping drm/tegra: Add plane support drm/tegra: Implement .mode_set_base() drm/tegra: Implement VBLANK support drm/tegra: Implement page-flipping support drm/tegra: Split DC_CMD_STATE_CONTROL register write drm/tegra: Add list of framebuffers to debugfs
Documentation/DocBook/drm.tmpl | 6 + drivers/gpu/drm/drm_crtc.c | 7 + drivers/gpu/drm/tegra/dc.c | 489 ++++++++++++++++++++++++++++++++--------- drivers/gpu/drm/tegra/dc.h | 2 +- drivers/gpu/drm/tegra/drm.c | 103 +++++++++ drivers/gpu/drm/tegra/drm.h | 37 ++++ 6 files changed, 533 insertions(+), 111 deletions(-)
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Mark Zhang -
Thierry Reding