Skip to contentSkip to conversation

some misc drm patches

PATCH In review Rob Clark 3 participants 10 messages Jan 9, 2013 – Jan 24, 2013

Subsystems (3)
lore
Share & tools
lore

Rob Clark

author
#0 lore raw

cover some misc drm patches

Misc fixes/enhancements that I did for TI LCDC DRM driver.

Rob Clark (4):
drm: small fix in drm_send_vblank_event()
drm/cma: add debugfs helpers
drm: i2c encoder helper wrappers
drm/nouveau: use i2c encoder helper wrappers

 drivers/gpu/drm/drm_encoder_slave.c  | 63 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_fb_cma_helper.c  | 53 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_gem_cma_helper.c | 21 ++++++++++++
 drivers/gpu/drm/drm_irq.c            |  1 +
 drivers/gpu/drm/nouveau/nv04_tv.c    | 39 ++++++++--------------
 include/drm/drm_encoder_slave.h      | 20 ++++++++++++
 include/drm/drm_fb_cma_helper.h      |  5 +++
 include/drm/drm_gem_cma_helper.h     |  4 +++
 8 files changed, 180 insertions(+), 26 deletions(-)
Signature · 1 line
1.8.0.2
author
#1 lore raw

PATCH 1/4 drm: small fix in drm_send_vblank_event()

Initialize e->pipe.. some drivers set this themselves, others do not. Setting it in drm_send_vblank_event() should help ensure more consistent behavior with the different drivers.

---
 drivers/gpu/drm/drm_irq.c | 1 +
 1 file changed, 1 insertion(+)
drivers/gpu/drm/drm_irq.c +1 -0
index 19c01ca..9bb83a5 100644
@@ -863,6 +863,7 @@ void drm_send_vblank_event(struct drm_device *dev, int crtc,
 
 		now = get_drm_timestamp();
 	}
+	e->pipe = crtc;
 	send_vblank_event(dev, e, seq, &now);
 }
 EXPORT_SYMBOL(drm_send_vblank_event);
Signature · 1 line
1.8.0.2
Reviewed-by on v1 1/4
#2 lore raw

Re: [PATCH 1/4] drm: small fix in drm_send_vblank_event()

Quoted 5 lines · Rob Clark

Initialize e->pipe.. some drivers set this themselves, others do not. Setting it in drm_send_vblank_event() should help ensure more consistent behavior with the different drivers.

Quoted 23 lines
---
 drivers/gpu/drm/drm_irq.c | 1 +
 1 file changed, 1 insertion(+)
drivers/gpu/drm/drm_irq.c +1 -1
index 19c01ca..9bb83a5 100644
@@ -863,6 +863,7 @@ void drm_send_vblank_event(struct drm_device *dev, int crtc,
 
                now = get_drm_timestamp();
        }
+       e->pipe = crtc;
        send_vblank_event(dev, e, seq, &now);
 }
 EXPORT_SYMBOL(drm_send_vblank_event);
--

1.8.0.2

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

author
#3 lore raw

PATCH 2/4 drm/cma: add debugfs helpers

In reply to Rob Clark

Add helper to display fb's which can be used directly in drm_info_list:

static struct drm_info_list foo_debugfs_list[] = {
		...
		{ "fb",   drm_fb_cma_debugfs_show, 0 },
};

to display information about CMA fb objects, as well as a drm_gem_cma_describe() which can be used if the driver bothers to keep a list of CMA GEM objects.

---
 drivers/gpu/drm/drm_fb_cma_helper.c  | 53 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_gem_cma_helper.c | 21 ++++++++++++++
 include/drm/drm_fb_cma_helper.h      |  5 ++++
 include/drm/drm_gem_cma_helper.h     |  4 +++
 4 files changed, 83 insertions(+)
drivers/gpu/drm/drm_fb_cma_helper.c +53 -0
index fd9d0af..18866ee 100644
@@ -180,6 +180,59 @@ struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb,
 }
 EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj);
 
+#ifdef CONFIG_DEBUG_FS
+/**
+ * drm_fb_cma_describe() - Helper to dump information about a single
+ * CMA framebuffer object
+ */
+void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
+{
+	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
+	int i, n = drm_format_num_planes(fb->pixel_format);
+
+	seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height,
+			(char *)&fb->pixel_format);
+
+	for (i = 0; i < n; i++) {
+		seq_printf(m, "   %d: offset=%d pitch=%d, obj: ",
+				i, fb->offsets[i], fb->pitches[i]);
+		drm_gem_cma_describe(fb_cma->obj[i], m);
+	}
+}
+EXPORT_SYMBOL_GPL(drm_fb_cma_describe);
+
+/**
+ * drm_fb_cma_debugfs_show() - Helper to list CMA framebuffer objects
+ * in debugfs.
+ */
+int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg)
+{
+	struct drm_info_node *node = (struct drm_info_node *) m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct drm_framebuffer *fb;
+	int ret;
+
+	ret = mutex_lock_interruptible(&dev->mode_config.mutex);
+	if (ret)
+		return ret;
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret) {
+		mutex_unlock(&dev->mode_config.mutex);
+		return ret;
+	}
+
+	list_for_each_entry(fb, &dev->mode_config.fb_list, head)
+		drm_fb_cma_describe(fb, m);
+
+	mutex_unlock(&dev->struct_mutex);
+	mutex_unlock(&dev->mode_config.mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_fb_cma_debugfs_show);
+#endif
+
 static struct fb_ops drm_fbdev_cma_ops = {
 	.owner		= THIS_MODULE,
 	.fb_fillrect	= sys_fillrect,
drivers/gpu/drm/drm_gem_cma_helper.c +21 -0
index 1aa8fee..0a7e011 100644
@@ -249,3 +249,24 @@ int drm_gem_cma_dumb_destroy(struct drm_file *file_priv,
 	return drm_gem_handle_delete(file_priv, handle);
 }
 EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_destroy);
+
+#ifdef CONFIG_DEBUG_FS
+void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj, struct seq_file *m)
+{
+	struct drm_gem_object *obj = &cma_obj->base;
+	struct drm_device *dev = obj->dev;
+	uint64_t off = 0;
+
+	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+
+	if (obj->map_list.map)
+		off = (uint64_t)obj->map_list.hash.key;
+
+	seq_printf(m, "%2d (%2d) %08llx %08Zx %p %d",
+			obj->name, obj->refcount.refcount.counter,
+			off, cma_obj->paddr, cma_obj->vaddr, obj->size);
+
+	seq_printf(m, "\n");
+}
+EXPORT_SYMBOL_GPL(drm_gem_cma_describe);
+#endif
include/drm/drm_fb_cma_helper.h +5 -0
index 76c7098..4a3fc24 100644
@@ -23,5 +23,10 @@ struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
 struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb,
 	unsigned int plane);
 
+#ifdef CONFIG_DEBUG_FS
+void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m);
+int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg);
+#endif
+
 #endif
 
include/drm/drm_gem_cma_helper.h +4 -0
index f0f6b1a..63397ce 100644
@@ -41,4 +41,8 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
 
 extern const struct vm_operations_struct drm_gem_cma_vm_ops;
 
+#ifdef CONFIG_DEBUG_FS
+void drm_gem_cma_describe(struct drm_gem_cma_object *obj, struct seq_file *m);
+#endif
+
 #endif /* __DRM_GEM_CMA_HELPER_H__ */
Signature · 1 line
1.8.0.2
#4 lore raw

Re: [PATCH 2/4] drm/cma: add debugfs helpers

Quoted 64 lines · Rob Clark

Add helper to display fb's which can be used directly in drm_info_list:

static struct drm_info_list foo_debugfs_list[] = {
		...
		{ "fb",   drm_fb_cma_debugfs_show, 0 },
};

to display information about CMA fb objects, as well as a drm_gem_cma_describe() which can be used if the driver bothers to keep a list of CMA GEM objects.

---
 drivers/gpu/drm/drm_fb_cma_helper.c  | 53 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_gem_cma_helper.c | 21 ++++++++++++++
 include/drm/drm_fb_cma_helper.h      |  5 ++++
 include/drm/drm_gem_cma_helper.h     |  4 +++
 4 files changed, 83 insertions(+)
drivers/gpu/drm/drm_fb_cma_helper.c +37 -0
index fd9d0af..18866ee 100644
@@ -180,6 +180,59 @@ struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb,
 }
 EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj);
 
+#ifdef CONFIG_DEBUG_FS
+/**
+ * drm_fb_cma_describe() - Helper to dump information about a single
+ * CMA framebuffer object
+ */
+void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
+{
+	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
+	int i, n = drm_format_num_planes(fb->pixel_format);
+
+	seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height,
+			(char *)&fb->pixel_format);
+
+	for (i = 0; i < n; i++) {
+		seq_printf(m, "   %d: offset=%d pitch=%d, obj: ",
+				i, fb->offsets[i], fb->pitches[i]);
+		drm_gem_cma_describe(fb_cma->obj[i], m);
+	}
+}
+EXPORT_SYMBOL_GPL(drm_fb_cma_describe);
+
+/**
+ * drm_fb_cma_debugfs_show() - Helper to list CMA framebuffer objects
+ * in debugfs.
+ */
+int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg)
+{
+	struct drm_info_node *node = (struct drm_info_node *) m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct drm_framebuffer *fb;
+	int ret;
+
+	ret = mutex_lock_interruptible(&dev->mode_config.mutex);
+	if (ret)
+		return ret;
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);

drivers/gpu/drm/drm_fb_cma_helper.c struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb, · line 229

Quoted 10 lines
drivers/gpu/drm/drm_fb_cma_helper.c +10 -0
+	if (ret) {
+		mutex_unlock(&dev->mode_config.mutex);
+		return ret;
+	}
+
+	list_for_each_entry(fb, &dev->mode_config.fb_list, head)
+		drm_fb_cma_describe(fb, m);
+
+	mutex_unlock(&dev->struct_mutex);
+	mutex_unlock(&dev->mode_config.mutex);

On top of my kms locking rework, this is way too much locking ;-) You should get by with dev->mode_config.fb_lock to prevent the fbs from disappearing. Everything else that drm_fb_cma_describe access should be invariant already, so doesn't need any locking.
-DAniel

Quoted 73 lines
drivers/gpu/drm/drm_fb_cma_helper.c +6 -0
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_fb_cma_debugfs_show);
+#endif
+
 static struct fb_ops drm_fbdev_cma_ops = {
 	.owner		= THIS_MODULE,
 	.fb_fillrect	= sys_fillrect,
drivers/gpu/drm/drm_gem_cma_helper.c +21 -0
index 1aa8fee..0a7e011 100644
@@ -249,3 +249,24 @@ int drm_gem_cma_dumb_destroy(struct drm_file *file_priv,
 	return drm_gem_handle_delete(file_priv, handle);
 }
 EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_destroy);
+
+#ifdef CONFIG_DEBUG_FS
+void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj, struct seq_file *m)
+{
+	struct drm_gem_object *obj = &cma_obj->base;
+	struct drm_device *dev = obj->dev;
+	uint64_t off = 0;
+
+	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+
+	if (obj->map_list.map)
+		off = (uint64_t)obj->map_list.hash.key;
+
+	seq_printf(m, "%2d (%2d) %08llx %08Zx %p %d",
+			obj->name, obj->refcount.refcount.counter,
+			off, cma_obj->paddr, cma_obj->vaddr, obj->size);
+
+	seq_printf(m, "\n");
+}
+EXPORT_SYMBOL_GPL(drm_gem_cma_describe);
+#endif
include/drm/drm_fb_cma_helper.h +5 -0
index 76c7098..4a3fc24 100644
@@ -23,5 +23,10 @@ struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
 struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb,
 	unsigned int plane);
 
+#ifdef CONFIG_DEBUG_FS
+void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m);
+int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg);
+#endif
+
 #endif
 
include/drm/drm_gem_cma_helper.h +4 -0
index f0f6b1a..63397ce 100644
@@ -41,4 +41,8 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
 
 extern const struct vm_operations_struct drm_gem_cma_vm_ops;
 
+#ifdef CONFIG_DEBUG_FS
+void drm_gem_cma_describe(struct drm_gem_cma_object *obj, struct seq_file *m);
+#endif
+
 #endif /* __DRM_GEM_CMA_HELPER_H__ */
Signature · 6 lines
1.8.0.2

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
author
#5 lore raw

PATCH 3/4 drm: i2c encoder helper wrappers

In reply to Rob Clark

Simplify life for drivers using an encoder-slave, so that they can make their drm_encoder_helper_funcs const, rather than needing to dynamically allocate and populate them.

---
 drivers/gpu/drm/drm_encoder_slave.c | 63 +++++++++++++++++++++++++++++++++++++
 include/drm/drm_encoder_slave.h     | 20 ++++++++++++
 2 files changed, 83 insertions(+)
drivers/gpu/drm/drm_encoder_slave.c +63 -0
index 63e7334..48c52f7 100644
@@ -123,3 +123,66 @@ void drm_i2c_encoder_destroy(struct drm_encoder *drm_encoder)
 	module_put(module);
 }
 EXPORT_SYMBOL(drm_i2c_encoder_destroy);
+
+/*
+ * Wrapper fxns which can be plugged in to drm_encoder_helper_funcs:
+ */
+
+static inline struct drm_encoder_slave_funcs *
+get_slave_funcs(struct drm_encoder *enc)
+{
+	return to_encoder_slave(enc)->slave_funcs;
+}
+
+void drm_i2c_encoder_dpms(struct drm_encoder *encoder, int mode)
+{
+	get_slave_funcs(encoder)->dpms(encoder, mode);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_dpms);
+
+bool drm_i2c_encoder_mode_fixup(struct drm_encoder *encoder,
+		const struct drm_display_mode *mode,
+		struct drm_display_mode *adjusted_mode)
+{
+	return get_slave_funcs(encoder)->mode_fixup(encoder, mode, adjusted_mode);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_mode_fixup);
+
+void drm_i2c_encoder_prepare(struct drm_encoder *encoder)
+{
+	drm_i2c_encoder_dpms(encoder, DRM_MODE_DPMS_OFF);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_prepare);
+
+void drm_i2c_encoder_commit(struct drm_encoder *encoder)
+{
+	drm_i2c_encoder_dpms(encoder, DRM_MODE_DPMS_ON);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_commit);
+
+void drm_i2c_encoder_mode_set(struct drm_encoder *encoder,
+		struct drm_display_mode *mode,
+		struct drm_display_mode *adjusted_mode)
+{
+	get_slave_funcs(encoder)->mode_set(encoder, mode, adjusted_mode);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_mode_set);
+
+enum drm_connector_status drm_i2c_encoder_detect(struct drm_encoder *encoder,
+	    struct drm_connector *connector)
+{
+	return get_slave_funcs(encoder)->detect(encoder, connector);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_detect);
+
+void drm_i2c_encoder_save(struct drm_encoder *encoder)
+{
+	get_slave_funcs(encoder)->save(encoder);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_save);
+
+void drm_i2c_encoder_restore(struct drm_encoder *encoder)
+{
+	get_slave_funcs(encoder)->restore(encoder);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_restore);
include/drm/drm_encoder_slave.h +20 -0
index b0c11a7..8b9cc36 100644
@@ -159,4 +159,24 @@ static inline void drm_i2c_encoder_unregister(struct drm_i2c_encoder_driver *dri
 
 void drm_i2c_encoder_destroy(struct drm_encoder *encoder);
 
+
+/*
+ * Wrapper fxns which can be plugged in to drm_encoder_helper_funcs:
+ */
+
+void drm_i2c_encoder_dpms(struct drm_encoder *encoder, int mode);
+bool drm_i2c_encoder_mode_fixup(struct drm_encoder *encoder,
+		const struct drm_display_mode *mode,
+		struct drm_display_mode *adjusted_mode);
+void drm_i2c_encoder_prepare(struct drm_encoder *encoder);
+void drm_i2c_encoder_commit(struct drm_encoder *encoder);
+void drm_i2c_encoder_mode_set(struct drm_encoder *encoder,
+		struct drm_display_mode *mode,
+		struct drm_display_mode *adjusted_mode);
+enum drm_connector_status drm_i2c_encoder_detect(struct drm_encoder *encoder,
+	    struct drm_connector *connector);
+void drm_i2c_encoder_save(struct drm_encoder *encoder);
+void drm_i2c_encoder_restore(struct drm_encoder *encoder);
+
+
 #endif
Signature · 1 line
1.8.0.2
Reviewed-by on v1 3/4
#6 lore raw

Re: [PATCH 3/4] drm: i2c encoder helper wrappers

Quoted 5 lines · Rob Clark

Simplify life for drivers using an encoder-slave, so that they can make their drm_encoder_helper_funcs const, rather than needing to dynamically allocate and populate them.

Ah, here they are - looked for them but didn't find them.

I guess if we keep on adding things to the encoder slave code (and more users) we should start thinking about some kerneldoc ... -Daniel

Quoted 112 lines
---
 drivers/gpu/drm/drm_encoder_slave.c | 63 +++++++++++++++++++++++++++++++++++++
 include/drm/drm_encoder_slave.h     | 20 ++++++++++++
 2 files changed, 83 insertions(+)
drivers/gpu/drm/drm_encoder_slave.c +63 -0
index 63e7334..48c52f7 100644
@@ -123,3 +123,66 @@ void drm_i2c_encoder_destroy(struct drm_encoder *drm_encoder)
 	module_put(module);
 }
 EXPORT_SYMBOL(drm_i2c_encoder_destroy);
+
+/*
+ * Wrapper fxns which can be plugged in to drm_encoder_helper_funcs:
+ */
+
+static inline struct drm_encoder_slave_funcs *
+get_slave_funcs(struct drm_encoder *enc)
+{
+	return to_encoder_slave(enc)->slave_funcs;
+}
+
+void drm_i2c_encoder_dpms(struct drm_encoder *encoder, int mode)
+{
+	get_slave_funcs(encoder)->dpms(encoder, mode);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_dpms);
+
+bool drm_i2c_encoder_mode_fixup(struct drm_encoder *encoder,
+		const struct drm_display_mode *mode,
+		struct drm_display_mode *adjusted_mode)
+{
+	return get_slave_funcs(encoder)->mode_fixup(encoder, mode, adjusted_mode);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_mode_fixup);
+
+void drm_i2c_encoder_prepare(struct drm_encoder *encoder)
+{
+	drm_i2c_encoder_dpms(encoder, DRM_MODE_DPMS_OFF);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_prepare);
+
+void drm_i2c_encoder_commit(struct drm_encoder *encoder)
+{
+	drm_i2c_encoder_dpms(encoder, DRM_MODE_DPMS_ON);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_commit);
+
+void drm_i2c_encoder_mode_set(struct drm_encoder *encoder,
+		struct drm_display_mode *mode,
+		struct drm_display_mode *adjusted_mode)
+{
+	get_slave_funcs(encoder)->mode_set(encoder, mode, adjusted_mode);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_mode_set);
+
+enum drm_connector_status drm_i2c_encoder_detect(struct drm_encoder *encoder,
+	    struct drm_connector *connector)
+{
+	return get_slave_funcs(encoder)->detect(encoder, connector);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_detect);
+
+void drm_i2c_encoder_save(struct drm_encoder *encoder)
+{
+	get_slave_funcs(encoder)->save(encoder);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_save);
+
+void drm_i2c_encoder_restore(struct drm_encoder *encoder)
+{
+	get_slave_funcs(encoder)->restore(encoder);
+}
+EXPORT_SYMBOL(drm_i2c_encoder_restore);
include/drm/drm_encoder_slave.h +20 -0
index b0c11a7..8b9cc36 100644
@@ -159,4 +159,24 @@ static inline void drm_i2c_encoder_unregister(struct drm_i2c_encoder_driver *dri
 
 void drm_i2c_encoder_destroy(struct drm_encoder *encoder);
 
+
+/*
+ * Wrapper fxns which can be plugged in to drm_encoder_helper_funcs:
+ */
+
+void drm_i2c_encoder_dpms(struct drm_encoder *encoder, int mode);
+bool drm_i2c_encoder_mode_fixup(struct drm_encoder *encoder,
+		const struct drm_display_mode *mode,
+		struct drm_display_mode *adjusted_mode);
+void drm_i2c_encoder_prepare(struct drm_encoder *encoder);
+void drm_i2c_encoder_commit(struct drm_encoder *encoder);
+void drm_i2c_encoder_mode_set(struct drm_encoder *encoder,
+		struct drm_display_mode *mode,
+		struct drm_display_mode *adjusted_mode);
+enum drm_connector_status drm_i2c_encoder_detect(struct drm_encoder *encoder,
+	    struct drm_connector *connector);
+void drm_i2c_encoder_save(struct drm_encoder *encoder);
+void drm_i2c_encoder_restore(struct drm_encoder *encoder);
+
+
 #endif
Signature · 6 lines
1.8.0.2

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
author
#7 lore raw

Re: [PATCH 3/4] drm: i2c encoder helper wrappers

Quoted 5 lines · Daniel Vetter
Quoted 4 lines · Rob Clark

Simplify life for drivers using an encoder-slave, so that they can make their drm_encoder_helper_funcs const, rather than needing to dynamically allocate and populate them.

Quoted 8 lines
Quoted 1 line

Ah, here they are - looked for them but didn't find them.

I guess if we keep on adding things to the encoder slave code (and more users) we should start thinking about some kerneldoc ...

yeah.. especially after it is renamed to cdf :-P

Quoted 2 lines

-Daniel

author
#8 lore raw

PATCH 4/4 drm/nouveau: use i2c encoder helper wrappers

In reply to Rob Clark
---
 drivers/gpu/drm/nouveau/nv04_tv.c | 39 +++++++++++++--------------------------
 1 file changed, 13 insertions(+), 26 deletions(-)
drivers/gpu/drm/nouveau/nv04_tv.c +13 -26
index 62e826a..4a69ccd 100644
@@ -184,14 +184,23 @@ static const struct drm_encoder_funcs nv04_tv_funcs = {
 	.destroy = nv04_tv_destroy,
 };
 
+static const struct drm_encoder_helper_funcs nv04_tv_helper_funcs = {
+	.dpms = nv04_tv_dpms,
+	.save = drm_i2c_encoder_save,
+	.restore = drm_i2c_encoder_restore,
+	.mode_fixup = drm_i2c_encoder_mode_fixup,
+	.prepare = nv04_tv_prepare,
+	.commit = nv04_tv_commit,
+	.mode_set = nv04_tv_mode_set,
+	.detect = drm_i2c_encoder_detect,
+};
+
 int
 nv04_tv_create(struct drm_connector *connector, struct dcb_output *entry)
 {
 	struct nouveau_encoder *nv_encoder;
 	struct drm_encoder *encoder;
 	struct drm_device *dev = connector->dev;
-	struct drm_encoder_helper_funcs *hfuncs;
-	struct drm_encoder_slave_funcs *sfuncs;
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct nouveau_i2c *i2c = nouveau_i2c(drm->device);
 	struct nouveau_i2c_port *port = i2c->find(i2c, entry->i2c_index);
@@ -207,17 +216,11 @@ nv04_tv_create(struct drm_connector *connector, struct dcb_output *entry)
 	if (!nv_encoder)
 		return -ENOMEM;
 
-	hfuncs = kzalloc(sizeof(*hfuncs), GFP_KERNEL);
-	if (!hfuncs) {
-		ret = -ENOMEM;
-		goto fail_free;
-	}
-
 	/* Initialize the common members */
 	encoder = to_drm_encoder(nv_encoder);
 
 	drm_encoder_init(dev, encoder, &nv04_tv_funcs, DRM_MODE_ENCODER_TVDAC);
-	drm_encoder_helper_add(encoder, hfuncs);
+	drm_encoder_helper_add(encoder, &nv04_tv_helper_funcs);
 
 	encoder->possible_crtcs = entry->heads;
 	encoder->possible_clones = 0;
@@ -230,30 +233,14 @@ nv04_tv_create(struct drm_connector *connector, struct dcb_output *entry)
 	if (ret < 0)
 		goto fail_cleanup;
 
-	/* Fill the function pointers */
-	sfuncs = get_slave_funcs(encoder);
-
-	*hfuncs = (struct drm_encoder_helper_funcs) {
-		.dpms = nv04_tv_dpms,
-		.save = sfuncs->save,
-		.restore = sfuncs->restore,
-		.mode_fixup = sfuncs->mode_fixup,
-		.prepare = nv04_tv_prepare,
-		.commit = nv04_tv_commit,
-		.mode_set = nv04_tv_mode_set,
-		.detect = sfuncs->detect,
-	};
-
 	/* Attach it to the specified connector. */
-	sfuncs->create_resources(encoder, connector);
+	get_slave_funcs(encoder)->create_resources(encoder, connector);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
 	return 0;
 
 fail_cleanup:
 	drm_encoder_cleanup(encoder);
-	kfree(hfuncs);
-fail_free:
 	kfree(nv_encoder);
 	return ret;
 }
Signature · 1 line
1.8.0.2
Reviewed-by on v1 4/4
#9 lore raw

Re: [PATCH 4/4] drm/nouveau: use i2c encoder helper wrappers

Quoted 1 line · Rob Clark

Yeah, this looks a lot better, and even fixes a small leak on module unload since the allocated func-table isn't freed apparently.

Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch