diff mbox

[PATCHv2,20/31] drm/omap: HDMI: Fix HSW value

Message ID 1456479379-6086-21-git-send-email-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tomi Valkeinen Feb. 26, 2016, 9:36 a.m. UTC
On OMAP4 and OMAP5 ES1.0 the HDMI_WP_VIDEO_TIMING_H:HSW field is
set directly to the HSW value. On later SoCs the field needs to be
programmed with the value of HSW-1.

Currently the driver always programs the field with the HSW value. Most
videomodes seem to work fine with that, but at least low resolution
interlaced modes don't work at all.

This patch fixes the HSW for OMAP5 ES2.0+ SoCs.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/hdmi_wp.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart Feb. 29, 2016, 9:55 p.m. UTC | #1
Hi Tomi,

Thank you for the patch.

On Friday 26 February 2016 11:36:08 Tomi Valkeinen wrote:
> On OMAP4 and OMAP5 ES1.0 the HDMI_WP_VIDEO_TIMING_H:HSW field is
> set directly to the HSW value. On later SoCs the field needs to be
> programmed with the value of HSW-1.
> 
> Currently the driver always programs the field with the HSW value. Most
> videomodes seem to work fine with that, but at least low resolution
> interlaced modes don't work at all.
> 
> This patch fixes the HSW for OMAP5 ES2.0+ SoCs.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/hdmi_wp.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
> b/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c index 7c544bc56fb5..48ffb39663c8
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
> @@ -165,12 +165,24 @@ void hdmi_wp_video_config_timing(struct hdmi_wp_data
> *wp, {
>  	u32 timing_h = 0;
>  	u32 timing_v = 0;
> +	bool hsw_minus_one = true;
> 
>  	DSSDBG("Enter hdmi_wp_video_config_timing\n");
> 
> +	/*
> +	 * On OMAP4 and OMAP5 ES1 the HSW field is programmed as is. On OMAP5
> +	 * ES2+ (including DRA7/AM5 SoCs) HSW field is programmed to hsw-1.
> +	 * However, we don't support OMAP5 ES1 at all, so we can just check for
> +	 * OMAP4 here.
> +	 */
> +	if (omapdss_get_version() == OMAPDSS_VER_OMAP4430_ES1 ||
> +	    omapdss_get_version() == OMAPDSS_VER_OMAP4430_ES2 ||
> +	    omapdss_get_version() == OMAPDSS_VER_OMAP4)
> +		hsw_minus_one = false;
> +
>  	timing_h |= FLD_VAL(timings->hbp, 31, 20);
>  	timing_h |= FLD_VAL(timings->hfp, 19, 8);
> -	timing_h |= FLD_VAL(timings->hsw, 7, 0);
> +	timing_h |= FLD_VAL(timings->hsw - (hsw_minus_one ? 1 : 0), 7, 0);

If you named the variable hsw_offset, make it an unsigned int, initialized it 
to 1 and set it to 0 instead of false, you could write this as

	timing_h |= FLD_VAL(timings->hsw - hsw_offset, 7, 0);

>  	hdmi_write_reg(wp->base, HDMI_WP_VIDEO_TIMING_H, timing_h);
> 
>  	timing_v |= FLD_VAL(timings->vbp, 31, 20);
Tomi Valkeinen March 1, 2016, 8:32 a.m. UTC | #2
On 29/02/16 23:55, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Friday 26 February 2016 11:36:08 Tomi Valkeinen wrote:
>> On OMAP4 and OMAP5 ES1.0 the HDMI_WP_VIDEO_TIMING_H:HSW field is
>> set directly to the HSW value. On later SoCs the field needs to be
>> programmed with the value of HSW-1.
>>
>> Currently the driver always programs the field with the HSW value. Most
>> videomodes seem to work fine with that, but at least low resolution
>> interlaced modes don't work at all.
>>
>> This patch fixes the HSW for OMAP5 ES2.0+ SoCs.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/dss/hdmi_wp.c | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
>> b/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c index 7c544bc56fb5..48ffb39663c8
>> 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
>> @@ -165,12 +165,24 @@ void hdmi_wp_video_config_timing(struct hdmi_wp_data
>> *wp, {
>>  	u32 timing_h = 0;
>>  	u32 timing_v = 0;
>> +	bool hsw_minus_one = true;
>>
>>  	DSSDBG("Enter hdmi_wp_video_config_timing\n");
>>
>> +	/*
>> +	 * On OMAP4 and OMAP5 ES1 the HSW field is programmed as is. On OMAP5
>> +	 * ES2+ (including DRA7/AM5 SoCs) HSW field is programmed to hsw-1.
>> +	 * However, we don't support OMAP5 ES1 at all, so we can just check for
>> +	 * OMAP4 here.
>> +	 */
>> +	if (omapdss_get_version() == OMAPDSS_VER_OMAP4430_ES1 ||
>> +	    omapdss_get_version() == OMAPDSS_VER_OMAP4430_ES2 ||
>> +	    omapdss_get_version() == OMAPDSS_VER_OMAP4)
>> +		hsw_minus_one = false;
>> +
>>  	timing_h |= FLD_VAL(timings->hbp, 31, 20);
>>  	timing_h |= FLD_VAL(timings->hfp, 19, 8);
>> -	timing_h |= FLD_VAL(timings->hsw, 7, 0);
>> +	timing_h |= FLD_VAL(timings->hsw - (hsw_minus_one ? 1 : 0), 7, 0);
> 
> If you named the variable hsw_offset, make it an unsigned int, initialized it 
> to 1 and set it to 0 instead of false, you could write this as

Thanks, that's a bit cleaner. I've made the change.

 Tomi
diff mbox

Patch

diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c b/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
index 7c544bc56fb5..48ffb39663c8 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi_wp.c
@@ -165,12 +165,24 @@  void hdmi_wp_video_config_timing(struct hdmi_wp_data *wp,
 {
 	u32 timing_h = 0;
 	u32 timing_v = 0;
+	bool hsw_minus_one = true;
 
 	DSSDBG("Enter hdmi_wp_video_config_timing\n");
 
+	/*
+	 * On OMAP4 and OMAP5 ES1 the HSW field is programmed as is. On OMAP5
+	 * ES2+ (including DRA7/AM5 SoCs) HSW field is programmed to hsw-1.
+	 * However, we don't support OMAP5 ES1 at all, so we can just check for
+	 * OMAP4 here.
+	 */
+	if (omapdss_get_version() == OMAPDSS_VER_OMAP4430_ES1 ||
+	    omapdss_get_version() == OMAPDSS_VER_OMAP4430_ES2 ||
+	    omapdss_get_version() == OMAPDSS_VER_OMAP4)
+		hsw_minus_one = false;
+
 	timing_h |= FLD_VAL(timings->hbp, 31, 20);
 	timing_h |= FLD_VAL(timings->hfp, 19, 8);
-	timing_h |= FLD_VAL(timings->hsw, 7, 0);
+	timing_h |= FLD_VAL(timings->hsw - (hsw_minus_one ? 1 : 0), 7, 0);
 	hdmi_write_reg(wp->base, HDMI_WP_VIDEO_TIMING_H, timing_h);
 
 	timing_v |= FLD_VAL(timings->vbp, 31, 20);