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

2012-01-06 Thread K, Mythri P 
Hi Tomi,


On Thu, Jan 5, 2012 at 12:51 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Wed, 2012-01-04 at 17:23 +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
 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   |   35 
 +++-
  drivers/video/omap2/dss/ti_hdmi.h      |    2 +
  5 files changed, 67 insertions(+), 1 deletions(-)

 diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
 index 6308fc5..cf1f0f9 100644
 --- a/drivers/video/omap2/dss/dss.h
 +++ b/drivers/video/omap2/dss/dss.h
 @@ -498,6 +498,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(int 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 b402699..c7e71b9 100644
 --- a/drivers/video/omap2/dss/dss_features.c
 +++ b/drivers/video/omap2/dss/dss_features.c
 @@ -465,6 +465,7 @@ static const struct ti_hdmi_ip_ops omap4_hdmi_functions 
 = {
       .dump_core              =       ti_hdmi_4xxx_core_dump,
       .dump_pll               =       ti_hdmi_4xxx_pll_dump,
       .dump_phy               =       ti_hdmi_4xxx_phy_dump,
 +     .configure_range        =       ti_hdmi_4xxx_configure_range,

  };

 diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
 index 4bb7678..ae7918e 100644
 --- a/drivers/video/omap2/dss/hdmi.c
 +++ b/drivers/video/omap2/dss/hdmi.c
 @@ -378,6 +378,34 @@ static void hdmi_power_off(struct omap_dss_device 
 *dssdev)
       hdmi_runtime_put();
  }

 +int omapdss_hdmi_set_range(int range)

 Range is an enum, not an int.

 +{
 +     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 == 0) 

 Range is an enum, not an int.

 +             ((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)

 Range is an enum, not an int... I won't comment on any more of these
 cases, please check all uses of range.

 +{
 +     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..c0aa922 100644
 --- a/drivers/video/omap2/dss/hdmi_panel.c
 +++ b/drivers/video/omap2/dss/hdmi_panel.c
 @@ -33,6 +33,33 @@ 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 = kstrtoul(buf, 0, range);
 +
 +     if (r || range  1)
 +             return -EINVAL;
 +
 +     r = omapdss_hdmi_set_range(range);
 +     if (r)
 +             return r;
 +
 +     return size;
 +}

 I don't like to add a new custom userspace API, but I guess we don't
 have much choice.

 However, I don't think using 0 and 1 in the API is very good. The
 choices with range should probably be full and limited.

 Btw, I tried to apply this patch set on top of dss master with and
 without the improve the timings... patch set, and failed both. What
 are these patches based on?

changes incorporated.
Thanks and regards,
Mythri.
  Tomi

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2012-01-04 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   |   35 +++-
 drivers/video/omap2/dss/ti_hdmi.h  |2 +
 5 files changed, 67 insertions(+), 1 deletions(-)

diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 6308fc5..cf1f0f9 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -498,6 +498,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(int 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 b402699..c7e71b9 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -465,6 +465,7 @@ static const struct ti_hdmi_ip_ops omap4_hdmi_functions = {
.dump_core  =   ti_hdmi_4xxx_core_dump,
.dump_pll   =   ti_hdmi_4xxx_pll_dump,
.dump_phy   =   ti_hdmi_4xxx_phy_dump,
+   .configure_range=   ti_hdmi_4xxx_configure_range,
 
 };
 
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 4bb7678..ae7918e 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -378,6 +378,34 @@ static void hdmi_power_off(struct omap_dss_device *dssdev)
hdmi_runtime_put();
 }
 
+int omapdss_hdmi_set_range(int 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 == 0) 
+   ((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..c0aa922 100644
--- a/drivers/video/omap2/dss/hdmi_panel.c
+++ b/drivers/video/omap2/dss/hdmi_panel.c
@@ -33,6 +33,33 @@ 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 = kstrtoul(buf, 0, range);
+
+   if (r || range  1)
+   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 +68,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 +84,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 1b485ee..1f15d74 100644
--- a/drivers/video/omap2/dss/ti_hdmi.h
+++ b/drivers/video/omap2/dss/ti_hdmi.h
@@ -111,6 +111,8 @@ struct ti_hdmi_ip_ops {
 
void (*dump_phy)(struct

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

2012-01-04 Thread Tomi Valkeinen 
On Wed, 2012-01-04 at 17:23 +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
 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   |   35 
 +++-
  drivers/video/omap2/dss/ti_hdmi.h  |2 +
  5 files changed, 67 insertions(+), 1 deletions(-)
 
 diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
 index 6308fc5..cf1f0f9 100644
 --- a/drivers/video/omap2/dss/dss.h
 +++ b/drivers/video/omap2/dss/dss.h
 @@ -498,6 +498,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(int 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 b402699..c7e71b9 100644
 --- a/drivers/video/omap2/dss/dss_features.c
 +++ b/drivers/video/omap2/dss/dss_features.c
 @@ -465,6 +465,7 @@ static const struct ti_hdmi_ip_ops omap4_hdmi_functions = 
 {
   .dump_core  =   ti_hdmi_4xxx_core_dump,
   .dump_pll   =   ti_hdmi_4xxx_pll_dump,
   .dump_phy   =   ti_hdmi_4xxx_phy_dump,
 + .configure_range=   ti_hdmi_4xxx_configure_range,
  
  };
  
 diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
 index 4bb7678..ae7918e 100644
 --- a/drivers/video/omap2/dss/hdmi.c
 +++ b/drivers/video/omap2/dss/hdmi.c
 @@ -378,6 +378,34 @@ static void hdmi_power_off(struct omap_dss_device 
 *dssdev)
   hdmi_runtime_put();
  }
  
 +int omapdss_hdmi_set_range(int range)

Range is an enum, not an int.

 +{
 + 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 == 0) 

Range is an enum, not an int.

 + ((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)

Range is an enum, not an int... I won't comment on any more of these
cases, please check all uses of range.

 +{
 + 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..c0aa922 100644
 --- a/drivers/video/omap2/dss/hdmi_panel.c
 +++ b/drivers/video/omap2/dss/hdmi_panel.c
 @@ -33,6 +33,33 @@ 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 = kstrtoul(buf, 0, range);
 +
 + if (r || range  1)
 + return -EINVAL;
 +
 + r = omapdss_hdmi_set_range(range);
 + if (r)
 + return r;
 +
 + return size;
 +}

I don't like to add a new custom userspace API, but I guess we don't
have much choice.

However, I don't think using 0 and 1 in the API is very good. The
choices with range should probably be full and limited.

Btw, I tried to apply this patch set on top of dss master with and
without the improve the timings... patch set, and failed both. What
are these patches based on?

 Tomi



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