Hi,
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 at codeaurora.org>
> ---
> 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.
> 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;
> mdp5_write(mdp5_kms, REG_MDP5_INTF_TIMING_ENGINE_EN(i), 0);
> + }
Writing to registers for a non existent interface shouldn't do any harm,
but it's best to be safe :)
> mdp5_disable(mdp5_kms);
> mdelay(16);
>
>
Thanks,
Archit
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
