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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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);

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel