Re: [PATCH 4/4] drm/msm/mdp5: Make the intf connection in config module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 


On 03/04/2015 09:14 PM, "Stéphane Viau" wrote:

Hi,


Hi Archit,



On 03/04/2015 12:06 AM, Stephane Viau wrote:

Up until now, we assume that eDP is tight to intf_0 and HDMI to
intf_3. This information shall actually come from the mdp5_cfg
module since it can change from one chip to another.

Signed-off-by: Stephane Viau <sviau@xxxxxxxxxxxxxx>
---
   drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c |   8 +++
   drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h |   4 ++
   drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 111
++++++++++++++++++--------------
   3 files changed, 74 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
index 72c075a..8bee023 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
@@ -62,6 +62,10 @@ const struct mdp5_cfg_hw msm8x74_config = {
   		.count = 4,
   		.base = { 0x12500, 0x12700, 0x12900, 0x12b00 },
   	},
+	.intfs = {
+		[0] = INTF_eDP,
+		[3] = INTF_HDMI,
+	},
   	.max_clk = 200000000,
   };

@@ -111,6 +115,10 @@ const struct mdp5_cfg_hw apq8084_config = {
   		.count = 5,
   		.base = { 0x12500, 0x12700, 0x12900, 0x12b00, 0x12d00 },
   	},
+	.intfs = {
+		[0] = INTF_eDP,
+		[3] = INTF_HDMI,
+	},
   	.max_clk = 320000000,
   };

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h
index be149b3..4e91f14 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h
@@ -58,6 +58,8 @@ struct mdp5_smp_block {
   	int reserved[MAX_CLIENTS];	/* # of MMBs allocated per client */
   };

+#define MDP5_INTF_NUM_MAX	5
+
   struct mdp5_cfg_hw {
   	char  *name;

@@ -71,6 +73,8 @@ struct mdp5_cfg_hw {
   	struct mdp5_sub_block ad;
   	struct mdp5_sub_block intf;

+	u32 intfs[MDP5_INTF_NUM_MAX]; /* array of enum mdp5_intf_type */
+


The array type could be "enum mdp5_intf_type" rather than u32.



The problem here is that mdp5_cfg.h must be included before mdp5.xml.h
(see mdp5_kms.h #24) so that mdp5_cfg pointer can be resolved in the
latter.
mdp5_cfg.h hence cannot use any types defined in mdp5.xml.h, including
'enum mdp5_intf_type'.


Ah okay.




   	uint32_t max_clk;
   };

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index 390d9d2..8cec00e 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -161,6 +161,44 @@ int mdp5_enable(struct mdp5_kms *mdp5_kms)
   	return 0;
   }

+static int construct_encoder(struct mdp5_kms *mdp5_kms,
+		enum mdp5_intf_type intf_type, int intf_num)
+{
+	struct drm_device *dev = mdp5_kms->dev;
+	struct msm_drm_private *priv = dev->dev_private;
+	struct drm_encoder *encoder;
+	struct mdp5_interface intf = {
+			.num	= intf_num,
+			.type	= intf_type,
+			.mode	= MDP5_INTF_MODE_NONE,
+	};
+	int ret = 0;
+
+	encoder = mdp5_encoder_init(dev, &intf);
+	if (IS_ERR(encoder)) {
+		ret = PTR_ERR(encoder);
+		dev_err(dev->dev, "failed to construct encoder: %d\n", ret);
+		return ret;
+	}
+
+	encoder->possible_crtcs = (1 << priv->num_crtcs) - 1;
+	priv->encoders[priv->num_encoders++] = encoder;
+
+	if (intf_type == INTF_HDMI) {
+		ret = hdmi_modeset_init(priv->hdmi, dev, encoder);
+		if (ret)
+			dev_err(dev->dev, "failed to init HDMI: %d\n", ret);
+
+	} else if (intf_type == INTF_eDP) {
+		/* Construct bridge/connector for eDP: */
+		ret = msm_edp_modeset_init(priv->edp, dev, encoder);
+		if (ret)
+			dev_err(dev->dev, "failed to init eDP: %d\n", ret);
+	}
+
+	return ret;
+}
+
   static int modeset_init(struct mdp5_kms *mdp5_kms)
   {
   	static const enum mdp5_pipe crtcs[] = {
@@ -171,7 +209,6 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
   	};
   	struct drm_device *dev = mdp5_kms->dev;
   	struct msm_drm_private *priv = dev->dev_private;
-	struct drm_encoder *encoder;
   	const struct mdp5_cfg_hw *hw_cfg;
   	int i, ret;

@@ -222,56 +259,29 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
   		}
   	}

-	if (priv->hdmi) {
-		struct mdp5_interface intf = {
-				.num	= 3,
-				.type	= INTF_HDMI,
-				.mode	= MDP5_INTF_MODE_NONE,
-		};
-
-		/* Construct encoder for HDMI: */
-		encoder = mdp5_encoder_init(dev, &intf);
-		if (IS_ERR(encoder)) {
-			dev_err(dev->dev, "failed to construct encoder\n");
-			ret = PTR_ERR(encoder);
-			goto fail;
-		}
-
-		encoder->possible_crtcs = (1 << priv->num_crtcs) - 1;;
-		priv->encoders[priv->num_encoders++] = encoder;
-
-		ret = hdmi_modeset_init(priv->hdmi, dev, encoder);
-		if (ret) {
-			dev_err(dev->dev, "failed to initialize HDMI: %d\n", ret);
-			goto fail;
-		}
-	}
-
-	if (priv->edp) {
-		struct mdp5_interface intf = {
-				.num	= 0,
-				.type	= INTF_eDP,
-				.mode	= MDP5_INTF_MODE_NONE,
-		};
-
-		/* Construct encoder for eDP: */
-		encoder = mdp5_encoder_init(dev, &intf);
-		if (IS_ERR(encoder)) {
-			dev_err(dev->dev, "failed to construct eDP encoder\n");
-			ret = PTR_ERR(encoder);
-			goto fail;
+	/* Construct external display interfaces' encoders: */
+	for (i = 0; i < ARRAY_SIZE(hw_cfg->intfs); i++) {
+		enum mdp5_intf_type intf_type = hw_cfg->intfs[i];
+
+		switch (intf_type) {
+		case INTF_DISABLED:
+			break;
+		case INTF_eDP:
+			if (priv->edp)
+				ret = construct_encoder(mdp5_kms, INTF_eDP, i);
+			break;
+		case INTF_HDMI:
+			if (priv->hdmi)
+				ret = construct_encoder(mdp5_kms, INTF_HDMI, i);
+			break;
+		default:
+			dev_err(dev->dev, "unknown intf: %d\n", intf_type);
+			ret = -EINVAL;
+			break;
   		}

-		encoder->possible_crtcs = (1 << priv->num_crtcs) - 1;
-		priv->encoders[priv->num_encoders++] = encoder;
-
-		/* Construct bridge/connector for eDP: */
-		ret = msm_edp_modeset_init(priv->edp, dev, encoder);
-		if (ret) {
-			dev_err(dev->dev, "failed to initialize eDP: %d\n",
-									ret);
+		if (ret)
   			goto fail;
-		}
   	}

   	return 0;
@@ -415,8 +425,11 @@ struct msm_kms *mdp5_kms_init(struct drm_device
*dev)
   	 * we don't disable):
   	 */
   	mdp5_enable(mdp5_kms);
-	for (i = 0; i < config->hw->intf.count; i++)
+	for (i = 0; i < MDP5_INTF_NUM_MAX; i++) {
+		if (MDP5_INTF_IS_VIRTUAL_DISPLAY(config->hw->intfs[i]))
+			continue;


This may not be sufficient to prevent register writes to interfaces that
don't exist. We'd probably need:

	if(MDP5_INTF_IS_VIRTUAL_DISPLAY(config->hw->intfs[i]) ||
			config->hw->intfs[i] == INTF_DISABLED)
		continue;



We actually need to disable all display interfaces even though they are
not listed as "enabled". If we do not do so, we get faults[1].
As you mentioned, it shouldn't harm to write 0 in non existent interfaces'
registers.

[1] http://www.hastebin.com/raw/dekaqaboju


Thanks for the clarification.

Archit

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux