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
Attachment:
pgpIyq5Tn8KdQ.pgp
Description: PGP signature
