[PATCH] OMAPDSS: VENC: allow switching venc type at runtime

VENC type (composite/svideo) doesn't have to be fixed by board wiring,
it is possible to provide both connectors, which is what pandora does.
Having to recompile the kernel for users who have TV connector types
that's don't match default board setting is very inconvenient, especially
for users of a consumer device, so add support for switching VENC type
at runtime over a new sysfs file venc_type.

Signed-off-by: Grazvydas Ignotas <redacted>
---
 Documentation/arm/OMAP/DSS     |    1 +
 drivers/video/omap2/dss/venc.c |   55 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 55 insertions(+), 1 deletions(-)

diff --git a/Documentation/arm/OMAP/DSS b/Documentation/arm/OMAP/DSS
index 888ae7b..18e2214 100644
--- a/Documentation/arm/OMAP/DSS
+++ b/Documentation/arm/OMAP/DSS

@@ -156,6 +156,7 @@ timings		Display timings (pixclock,xres/hfp/hbp/hsw,yres/vfp/vbp/vsw)
 		"pal" and "ntsc"
 panel_name
 tear_elim	Tearing elimination 0=off, 1=on
+venc_type	Output type (video encoder only): "composite" or "svideo"
 
 There are also some debugfs files at <debugfs>/omapdss/ which show information
 about clocks and registers.

diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index 9c3daf7..aa2e74a 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c

@@ -485,16 +485,69 @@ unsigned long venc_get_pixel_clock(void)
 	return 13500000;
 }
 
+static ssize_t display_venc_type_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct omap_dss_device *dssdev = to_dss_device(dev);
+	const char *ret;
+
+	switch (dssdev->phy.venc.type) {
+	case OMAP_DSS_VENC_TYPE_COMPOSITE:
+		ret = "composite";
+		break;
+	case OMAP_DSS_VENC_TYPE_SVIDEO:
+		ret = "svideo";
+		break;
+	default:
+		ret = "unknown";
+		break;
+	}
+
+	return snprintf(buf, PAGE_SIZE, "%s\n", ret);
+}
+
+static ssize_t display_venc_type_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	struct omap_dss_device *dssdev = to_dss_device(dev);
+	enum omap_dss_venc_type new_type;
+
+	if (strncmp("composite", buf, 9) = 0)
+		new_type = OMAP_DSS_VENC_TYPE_COMPOSITE;
+	else if (strncmp("svideo", buf, 6) = 0)
+		new_type = OMAP_DSS_VENC_TYPE_SVIDEO;
+	else
+		return -EINVAL;
+
+	mutex_lock(&venc.venc_lock);
+
+	if (dssdev->phy.venc.type != new_type) {
+		dssdev->phy.venc.type = new_type;
+		if (dssdev->state = OMAP_DSS_DISPLAY_ACTIVE) {
+			venc_power_off(dssdev);
+			venc_power_on(dssdev);
+		}
+	}
+
+	mutex_unlock(&venc.venc_lock);
+
+	return size;
+}
+
+static DEVICE_ATTR(venc_type, S_IRUGO | S_IWUSR,
+		display_venc_type_show, display_venc_type_store);
+
 /* driver */
 static int venc_panel_probe(struct omap_dss_device *dssdev)
 {
 	dssdev->panel.timings = omap_dss_pal_timings;
 
-	return 0;
+	return device_create_file(&dssdev->dev, &dev_attr_venc_type);
 }
 
 static void venc_panel_remove(struct omap_dss_device *dssdev)
 {
+	device_remove_file(&dssdev->dev, &dev_attr_venc_type);
 }
 
 static int venc_panel_enable(struct omap_dss_device *dssdev)

-- 
1.7.0.4

On Thu, 2012-03-29 at 02:45 +0300, Grazvydas Ignotas wrote:

VENC type (composite/svideo) doesn't have to be fixed by board wiring,
it is possible to provide both connectors, which is what pandora does.
Having to recompile the kernel for users who have TV connector types
that's don't match default board setting is very inconvenient, especially

You don't have to recompile the kernel, you could just set the venc type
in the board file depending on a boot parameter.

for users of a consumer device, so add support for switching VENC type
at runtime over a new sysfs file venc_type.

I really dislike adding new custom sysfs entries for omapdss, and I'd
like to avoid them if at all possible. Do you need to change the venc
type during runtime, or is it enough that it can be set during boot?

 Tomi

On Fri, Apr 20, 2012 at 11:38 AM, Tomi Valkeinen [off-list ref] wrote:

On Thu, 2012-03-29 at 02:45 +0300, Grazvydas Ignotas wrote:

quoted

VENC type (composite/svideo) doesn't have to be fixed by board wiring,
it is possible to provide both connectors, which is what pandora does.
Having to recompile the kernel for users who have TV connector types
that's don't match default board setting is very inconvenient, especially

You don't have to recompile the kernel, you could just set the venc type
in the board file depending on a boot parameter.

quoted

for users of a consumer device, so add support for switching VENC type
at runtime over a new sysfs file venc_type.

I really dislike adding new custom sysfs entries for omapdss, and I'd
like to avoid them if at all possible.

Well some panels already have custom attributes, and venc could be
considered as special panel type, so if it's allowed for panels, why
not allow it for venc?

Do you need to change the venc
type during runtime, or is it enough that it can be set during boot?

We need this on runtime, otherwise it causes several issues:
- reboot is required to change the setting, although there is no
technical reason to really require it. This punishes users who want to
try both settings or have both TV types (with a portable device this
may sometimes happen).
- having to provide a way for users to change this in kernel boot
arguments. Note that many pandora users don't know how to handle boot
scripts, so a bootloader menu of some sort would be needed or ability
to edit u-boot environment from Linux, both of which would be
needlessly complicated solutions.


-- 
Gražvydas

On Fri, 2012-04-20 at 13:49 +0300, Grazvydas Ignotas wrote:

On Fri, Apr 20, 2012 at 11:38 AM, Tomi Valkeinen [off-list ref] wrote:

quoted

On Thu, 2012-03-29 at 02:45 +0300, Grazvydas Ignotas wrote:

quoted

VENC type (composite/svideo) doesn't have to be fixed by board wiring,
it is possible to provide both connectors, which is what pandora does.
Having to recompile the kernel for users who have TV connector types
that's don't match default board setting is very inconvenient, especially

You don't have to recompile the kernel, you could just set the venc type
in the board file depending on a boot parameter.

quoted

for users of a consumer device, so add support for switching VENC type
at runtime over a new sysfs file venc_type.

I really dislike adding new custom sysfs entries for omapdss, and I'd
like to avoid them if at all possible.

Well some panels already have custom attributes, and venc could be
considered as special panel type, so if it's allowed for panels, why
not allow it for venc?

It's not really about "allowing". It's just that each new sysfs file is
a new non-standard custom API to userspace which we need to support
until the end of time. Adding new sysfs files carelessly will cause a
nightmare for me in the future, so by default I'm against new sysfs
files =).

quoted

Do you need to change the venc
type during runtime, or is it enough that it can be set during boot?

We need this on runtime, otherwise it causes several issues:
- reboot is required to change the setting, although there is no
technical reason to really require it. This punishes users who want to
try both settings or have both TV types (with a portable device this
may sometimes happen).
- having to provide a way for users to change this in kernel boot
arguments. Note that many pandora users don't know how to handle boot
scripts, so a bootloader menu of some sort would be needed or ability
to edit u-boot environment from Linux, both of which would be
needlessly complicated solutions.

Ok. Sounds like we need to have dynamic configuration. I'll review the
patch.

 Tomi

On Thu, 2012-03-29 at 02:45 +0300, Grazvydas Ignotas wrote:

I think we could have a better name here. "venc" name is quite obscure
on the user level. And it's not even quite correct, venc stays the same,
it's just the output that is changed. "output_type"? "connector"?
"connector_type"?

Would it be better to return an error here? It would be nice to have
matching input and output for the sysfs file. And generally speaking,
the default branch should never happen.

+
+	return snprintf(buf, PAGE_SIZE, "%s\n", ret);
+}
+
+static ssize_t display_venc_type_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	struct omap_dss_device *dssdev = to_dss_device(dev);
+	enum omap_dss_venc_type new_type;
+
+	if (strncmp("composite", buf, 9) == 0)
+		new_type = OMAP_DSS_VENC_TYPE_COMPOSITE;
+	else if (strncmp("svideo", buf, 6) == 0)
+		new_type = OMAP_DSS_VENC_TYPE_SVIDEO;

Here you could use sysfs_streq().

 Tomi

On Mon, Apr 23, 2012 at 3:23 PM, Tomi Valkeinen [off-list ref] wrote:

On Thu, 2012-03-29 at 02:45 +0300, Grazvydas Ignotas wrote:

quoted

VENC type (composite/svideo) doesn't have to be fixed by board wiring,
it is possible to provide both connectors, which is what pandora does.
Having to recompile the kernel for users who have TV connector types
that's don't match default board setting is very inconvenient, especially
for users of a consumer device, so add support for switching VENC type
at runtime over a new sysfs file venc_type.

Signed-off-by: Grazvydas Ignotas <redacted>
---
 Documentation/arm/OMAP/DSS     |    1 +
 drivers/video/omap2/dss/venc.c |   55 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 55 insertions(+), 1 deletions(-)

diff --git a/Documentation/arm/OMAP/DSS b/Documentation/arm/OMAP/DSS
index 888ae7b..18e2214 100644
--- a/Documentation/arm/OMAP/DSS
+++ b/Documentation/arm/OMAP/DSS

@@ -156,6 +156,7 @@ timings           Display timings (pixclock,xres/hfp/hbp/hsw,yres/vfp/vbp/vsw)

              "pal" and "ntsc"
 panel_name
 tear_elim    Tearing elimination 0=off, 1=on
+venc_type    Output type (video encoder only): "composite" or "svideo"

I think we could have a better name here. "venc" name is quite obscure
on the user level. And it's not even quite correct, venc stays the same,
it's just the output that is changed. "output_type"? "connector"?
"connector_type"?

I'll go for output_type then, connector* associates with fixed hardware to me.
I'll take care of other comments too.


-- 
Gražvydas