linux-kernel - Re: [PATCH 2/2] drm/msm: Add the eDP connector in msm drm driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141208133431.GB13942@ulmo.nvidia.com>
Date:	Mon, 8 Dec 2014 14:34:33 +0100
From:	Thierry Reding <thierry.reding@...il.com>
To:	Hai Li <hali@...eaurora.org>
Cc:	dri-devel@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
	linux-kernel@...r.kernel.org, robdclark@...il.com
Subject: Re: [PATCH 2/2] drm/msm: Add the eDP connector in msm drm driver

On Fri, Dec 05, 2014 at 04:30:01PM -0500, Hai Li wrote:
> Modified the hard-coded hdmi connector/encoder implementations in msm drm
> driver to support both edp and hdmi.

s/hdmi/HDMI/, s/msm/MSM/, s/drm/DRM/, s/edp/eDP/

[...]
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c
[...]
> @@ -177,7 +180,28 @@ static void mdp5_encoder_mode_set(struct drm_encoder *encoder,
>  	/* probably need to get DATA_EN polarity from panel.. */
>  
>  	dtv_hsync_skew = 0;  /* get this from panel? */
> -	format = 0x213f;     /* get this from panel? */
> +
> +	/* Get color format from panel, default is 8bpc */
> +	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> +		if (connector->encoder == encoder) {
> +			switch (connector->display_info.bpc) {
> +			case 4:
> +				format |= 0;
> +				break;
> +			case 5:
> +				format |= 0x15;
> +				break;
> +			case 6:
> +				format |= 0x2A;
> +				break;
> +			case 8:
> +			default:
> +				format |= 0x3F;
> +				break;
> +			}
> +			break;

I suppose that it might be obvious from the above switch statement, but
having symbolic names for the values of format might still be good for
readability.

> +		}
> +	}
>  
>  	hsync_start_x = (mode->htotal - mode->hsync_start);
>  	hsync_end_x = mode->htotal - (mode->hsync_start - mode->hdisplay) - 1;
> @@ -187,6 +211,16 @@ static void mdp5_encoder_mode_set(struct drm_encoder *encoder,
>  	display_v_start = (mode->vtotal - mode->vsync_start) * mode->htotal + dtv_hsync_skew;
>  	display_v_end = vsync_period - ((mode->vsync_start - mode->vdisplay) * mode->htotal) + dtv_hsync_skew - 1;
>  
> +	/*
> +	 * For edp only:
> +	 * DISPLAY_V_START = (VBP * HCYCLE) + HBP
> +	 * DISPLAY_V_END = (VBP + VACTIVE) * HCYCLE - 1 - HFP
> +	 */
> +	if (mdp5_encoder->intf_id == INTF_eDP) {
> +		display_v_start += (mode->htotal - mode->hsync_start);
> +		display_v_end -= (mode->hsync_start - mode->hdisplay);

No need for the parentheses.

> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
[...]
>  	struct drm_device *dev = mdp5_kms->dev;
>  	struct msm_drm_private *priv = dev->dev_private;
> -	struct drm_encoder *encoder;
> +	struct drm_encoder *edp_encoder = NULL, *hdmi_encoder = NULL;

Can't you simply reuse encoder? It's scope is limited to the
conditionals, so no need to have two separate variables.

Thierry

Content of type "application/pgp-signature" skipped