| Message ID | 1425407775-7704-5-git-send-email-sviau@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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@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
> 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@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. > 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'. >> 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 >> 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 > -- > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" > in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Thanks, Stéphane.
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@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. >> > > 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
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 */ + 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; mdp5_write(mdp5_kms, REG_MDP5_INTF_TIMING_ENGINE_EN(i), 0); + } mdp5_disable(mdp5_kms); mdelay(16);
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@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(-)