Re: [PATCH v3 3/3] OMAPDSS: HDMI: Sysfs support to configure quantization range

2012-01-08 Thread Tomi Valkeinen
On Fri, 2012-01-06 at 17:58 +0530, mythr...@ti.com wrote:
 From: Mythri P K mythr...@ti.com
 
 Add sysfs support for the uset space to configure limited range or full range

User typoed.

 quantization for HDMI.
 
 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/dss.h  |2 +
  drivers/video/omap2/dss/dss_features.c |1 +
  drivers/video/omap2/dss/hdmi.c |   28 +++
  drivers/video/omap2/dss/hdmi_panel.c   |   39 
 +++-
  drivers/video/omap2/dss/ti_hdmi.h  |2 +
  5 files changed, 71 insertions(+), 1 deletions(-)
 
 diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
 index 3cf99a9..0b1b6f2 100644
 --- a/drivers/video/omap2/dss/dss.h
 +++ b/drivers/video/omap2/dss/dss.h
 @@ -518,6 +518,8 @@ int omapdss_hdmi_display_check_timing(struct 
 omap_dss_device *dssdev,
   struct omap_video_timings *timings);
  int omapdss_hdmi_read_edid(u8 *buf, int len);
  bool omapdss_hdmi_detect(void);
 +int omapdss_hdmi_get_range(void);
 +int omapdss_hdmi_set_range(enum hdmi_range range);
  int hdmi_panel_init(void);
  void hdmi_panel_exit(void);
  
 diff --git a/drivers/video/omap2/dss/dss_features.c 
 b/drivers/video/omap2/dss/dss_features.c
 index afcb593..544b172 100644
 --- a/drivers/video/omap2/dss/dss_features.c
 +++ b/drivers/video/omap2/dss/dss_features.c
 @@ -476,6 +476,7 @@ static const struct ti_hdmi_ip_ops omap4_hdmi_functions = 
 {
   defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
   .audio_enable   =   ti_hdmi_4xxx_wp_audio_enable,
  #endif
 + .configure_range=   ti_hdmi_4xxx_configure_range,
  
  };
  
 diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
 index 92a6679..65397f7 100644
 --- a/drivers/video/omap2/dss/hdmi.c
 +++ b/drivers/video/omap2/dss/hdmi.c
 @@ -384,6 +384,34 @@ static void hdmi_power_off(struct omap_dss_device 
 *dssdev)
   hdmi_runtime_put();
  }
  
 +int omapdss_hdmi_set_range(enum hdmi_range range)
 +{
 + int r = 0;
 + enum hdmi_range old_range;
 +
 + old_range = hdmi.ip_data.range;
 + hdmi.ip_data.range = range;
 +
 + /* HDMI 1.3 section 6.6 VGA (640x480) format requires Full Range */
 + if ((range == HDMI_LIMITED_RANGE) 
 + ((hdmi.ip_data.cfg.cm.code == 4 
 + hdmi.ip_data.cfg.cm.mode == HDMI_DVI) ||
 + (hdmi.ip_data.cfg.cm.code == 1 
 + hdmi.ip_data.cfg.cm.mode == HDMI_HDMI)))
 + return -EINVAL;
 +
 + r = hdmi.ip_data.ops-configure_range(hdmi.ip_data);
 + if (r)
 + hdmi.ip_data.range = old_range;
 +
 + return r;
 +}

The error handling here doesn't look valid. The VGA case doesn't restore
the old_range, and generally speaking, storing the old value and
restoring it in the error case is not very nice.

You should first do the argument checks, and then call the
configure_range(), and if both succeed, only then set the
hdmi.ip_data.range. For this you need to pass the range parameter to
configure_range().

 +int omapdss_hdmi_get_range(void)

hdmi_range is an enum, not an int.

 +{
 + return hdmi.ip_data.range;
 +}
 +
  int omapdss_hdmi_display_check_timing(struct omap_dss_device *dssdev,
   struct omap_video_timings *timings)
  {
 diff --git a/drivers/video/omap2/dss/hdmi_panel.c 
 b/drivers/video/omap2/dss/hdmi_panel.c
 index 533d5dc..3166602 100644
 --- a/drivers/video/omap2/dss/hdmi_panel.c
 +++ b/drivers/video/omap2/dss/hdmi_panel.c
 @@ -33,6 +33,37 @@ static struct {
   struct mutex hdmi_lock;
  } hdmi;
  
 +static ssize_t hdmi_range_show(struct device *dev,
 + struct device_attribute *attr, char *buf)
 +{
 + int r;
 +
 + r = omapdss_hdmi_get_range();

hdmi_range is an enum, not an int.

 + return snprintf(buf, PAGE_SIZE, %d\n, r);
 +}
 +
 +static ssize_t hdmi_range_store(struct device *dev,
 + struct device_attribute *attr,
 + const char *buf, size_t size)
 +{
 + unsigned long range;
 + int r ;
 +
 + if (strncmp(limited, buf, 7) == 0)
 + range = 0;
 + else if (strncmp(full, buf, 4) == 0)
 + range = 1;
 + else
 + return -EINVAL;
 +
 + r = omapdss_hdmi_set_range(range);

hdmi_range is an enum, not an unsigned long.

 Tomi



signature.asc
Description: This is a digitally signed message part


[PATCH v3 3/3] OMAPDSS: HDMI: Sysfs support to configure quantization range

2012-01-06 Thread mythripk
From: Mythri P K mythr...@ti.com

Add sysfs support for the uset space to configure limited range or full range
quantization for HDMI.

Signed-off-by: Mythri P K mythr...@ti.com
---
 drivers/video/omap2/dss/dss.h  |2 +
 drivers/video/omap2/dss/dss_features.c |1 +
 drivers/video/omap2/dss/hdmi.c |   28 +++
 drivers/video/omap2/dss/hdmi_panel.c   |   39 +++-
 drivers/video/omap2/dss/ti_hdmi.h  |2 +
 5 files changed, 71 insertions(+), 1 deletions(-)

diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 3cf99a9..0b1b6f2 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -518,6 +518,8 @@ int omapdss_hdmi_display_check_timing(struct 
omap_dss_device *dssdev,
struct omap_video_timings *timings);
 int omapdss_hdmi_read_edid(u8 *buf, int len);
 bool omapdss_hdmi_detect(void);
+int omapdss_hdmi_get_range(void);
+int omapdss_hdmi_set_range(enum hdmi_range range);
 int hdmi_panel_init(void);
 void hdmi_panel_exit(void);
 
diff --git a/drivers/video/omap2/dss/dss_features.c 
b/drivers/video/omap2/dss/dss_features.c
index afcb593..544b172 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -476,6 +476,7 @@ static const struct ti_hdmi_ip_ops omap4_hdmi_functions = {
defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
.audio_enable   =   ti_hdmi_4xxx_wp_audio_enable,
 #endif
+   .configure_range=   ti_hdmi_4xxx_configure_range,
 
 };
 
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 92a6679..65397f7 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -384,6 +384,34 @@ static void hdmi_power_off(struct omap_dss_device *dssdev)
hdmi_runtime_put();
 }
 
+int omapdss_hdmi_set_range(enum hdmi_range range)
+{
+   int r = 0;
+   enum hdmi_range old_range;
+
+   old_range = hdmi.ip_data.range;
+   hdmi.ip_data.range = range;
+
+   /* HDMI 1.3 section 6.6 VGA (640x480) format requires Full Range */
+   if ((range == HDMI_LIMITED_RANGE) 
+   ((hdmi.ip_data.cfg.cm.code == 4 
+   hdmi.ip_data.cfg.cm.mode == HDMI_DVI) ||
+   (hdmi.ip_data.cfg.cm.code == 1 
+   hdmi.ip_data.cfg.cm.mode == HDMI_HDMI)))
+   return -EINVAL;
+
+   r = hdmi.ip_data.ops-configure_range(hdmi.ip_data);
+   if (r)
+   hdmi.ip_data.range = old_range;
+
+   return r;
+}
+
+int omapdss_hdmi_get_range(void)
+{
+   return hdmi.ip_data.range;
+}
+
 int omapdss_hdmi_display_check_timing(struct omap_dss_device *dssdev,
struct omap_video_timings *timings)
 {
diff --git a/drivers/video/omap2/dss/hdmi_panel.c 
b/drivers/video/omap2/dss/hdmi_panel.c
index 533d5dc..3166602 100644
--- a/drivers/video/omap2/dss/hdmi_panel.c
+++ b/drivers/video/omap2/dss/hdmi_panel.c
@@ -33,6 +33,37 @@ static struct {
struct mutex hdmi_lock;
 } hdmi;
 
+static ssize_t hdmi_range_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   int r;
+
+   r = omapdss_hdmi_get_range();
+   return snprintf(buf, PAGE_SIZE, %d\n, r);
+}
+
+static ssize_t hdmi_range_store(struct device *dev,
+   struct device_attribute *attr,
+   const char *buf, size_t size)
+{
+   unsigned long range;
+   int r ;
+
+   if (strncmp(limited, buf, 7) == 0)
+   range = 0;
+   else if (strncmp(full, buf, 4) == 0)
+   range = 1;
+   else
+   return -EINVAL;
+
+   r = omapdss_hdmi_set_range(range);
+   if (r)
+   return r;
+
+   return size;
+}
+
+static DEVICE_ATTR(range, S_IRUGO | S_IWUSR, hdmi_range_show, 
hdmi_range_store);
 
 static int hdmi_panel_probe(struct omap_dss_device *dssdev)
 {
@@ -41,6 +72,12 @@ static int hdmi_panel_probe(struct omap_dss_device *dssdev)
dssdev-panel.config = OMAP_DSS_LCD_TFT |
OMAP_DSS_LCD_IVS | OMAP_DSS_LCD_IHS;
 
+   /* sysfs entry to provide user space control to set
+* quantization range
+*/
+   if (device_create_file(dssdev-dev, dev_attr_range))
+   DSSERR(failed to create sysfs file\n);
+
dssdev-panel.timings = (struct omap_video_timings){640, 480, 25175, 
96, 16, 48, 2 , 11, 31};
 
DSSDBG(hdmi_panel_probe x_res= %d y_res = %d\n,
@@ -51,7 +88,7 @@ static int hdmi_panel_probe(struct omap_dss_device *dssdev)
 
 static void hdmi_panel_remove(struct omap_dss_device *dssdev)
 {
-
+   device_remove_file(dssdev-dev, dev_attr_range);
 }
 
 static int hdmi_panel_enable(struct omap_dss_device *dssdev)
diff --git a/drivers/video/omap2/dss/ti_hdmi.h 
b/drivers/video/omap2/dss/ti_hdmi.h
index ab0f2c2..9a31683 100644
--- a/drivers/video/omap2/dss/ti_hdmi.h
+++