Hi Tomi,
On Thu, Jan 5, 2012 at 12:51 PM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
> On Wed, 2012-01-04 at 17:23 +0530, mythripk@xxxxxx wrote:
>> From: Mythri P K <mythripk@xxxxxx>
>>
>> Add sysfs support for the uset space to configure limited range or full range
>> quantization for HDMI.
>>
>> Signed-off-by: Mythri P K <mythripk@xxxxxx>
>> ---
>> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
