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

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2966 bytes --]

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
> 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 <notasas@gmail.com>
> ---
>  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"?

>  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;
> +	}

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


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]