From: abhinavk@codeaurora.org
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>,
Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>,
Jonathan Marek <jonathan@marek.ca>,
Stephen Boyd <sboyd@kernel.org>, David Airlie <airlied@linux.ie>,
Daniel Vetter <daniel@ffwll.ch>,
linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
freedreno@lists.freedesktop.org
Subject: Re: [PATCH v3 3/7] drm/msm/dpu: support setting up two independent DSI connectors
Date: Mon, 12 Jul 2021 14:15:32 -0700 [thread overview]
Message-ID: <2b7677a4acfa1ab8ba5d67a69fea9e07@codeaurora.org> (raw)
In-Reply-To: <20210710222005.1334734-4-dmitry.baryshkov@linaro.org>
On 2021-07-10 15:20, Dmitry Baryshkov wrote:
> Move setting up encoders from set_encoder_mode to
> _dpu_kms_initialize_dsi() / _dpu_kms_initialize_displayport(). This
> allows us to support not only "single DSI" and "bonded DSI" but also
> "two
> independent DSI" configurations. In future this would also help adding
> support for multiple DP connectors.
>
This looks quite neat now,so i am okay with this version of it:
Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
Just a suggestion, since we are only supporting two dsis so far, do we
need
an extra variable to get the other DSI? Can't we just do
priv->dsi[DSI_1]?
as usually DSI_0 is the master
> + int other = (i + 1) % 2;
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 102 +++++++++++++-----------
> 1 file changed, 57 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 1d3a4f395e74..3cd2011e18d4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -471,30 +471,68 @@ static int _dpu_kms_initialize_dsi(struct
> drm_device *dev,
> struct dpu_kms *dpu_kms)
> {
> struct drm_encoder *encoder = NULL;
> + struct msm_display_info info;
> int i, rc = 0;
>
> if (!(priv->dsi[0] || priv->dsi[1]))
> return rc;
>
> - /*TODO: Support two independent DSI connectors */
> - encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
> - if (IS_ERR(encoder)) {
> - DPU_ERROR("encoder init failed for dsi display\n");
> - return PTR_ERR(encoder);
> - }
> -
> - priv->encoders[priv->num_encoders++] = encoder;
> -
> + /*
> + * We support following confiurations:
> + * - Single DSI host (dsi0 or dsi1)
> + * - Two independent DSI hosts
> + * - Bonded DSI0 and DSI1 hosts
> + *
> + * TODO: Support swapping DSI0 and DSI1 in the bonded setup.
> + */
> for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
> + int other = (i + 1) % 2;
> +
> if (!priv->dsi[i])
> continue;
>
> + if (msm_dsi_is_bonded_dsi(priv->dsi[i]) &&
> + !msm_dsi_is_master_dsi(priv->dsi[i]))
> + continue;
> +
> + encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
> + if (IS_ERR(encoder)) {
> + DPU_ERROR("encoder init failed for dsi display\n");
> + return PTR_ERR(encoder);
> + }
> +
> + priv->encoders[priv->num_encoders++] = encoder;
> +
> + memset(&info, 0, sizeof(info));
> + info.intf_type = encoder->encoder_type;
> +
> rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
> if (rc) {
> DPU_ERROR("modeset_init failed for dsi[%d], rc = %d\n",
> i, rc);
> break;
> }
> +
> + info.h_tile_instance[info.num_of_h_tiles++] = i;
> + info.capabilities = msm_dsi_is_cmd_mode(priv->dsi[i]) ?
> + MSM_DISPLAY_CAP_CMD_MODE :
> + MSM_DISPLAY_CAP_VID_MODE;
> +
> + if (msm_dsi_is_bonded_dsi(priv->dsi[i]) && priv->dsi[other]) {
> + rc = msm_dsi_modeset_init(priv->dsi[other], dev, encoder);
> + if (rc) {
> + DPU_ERROR("modeset_init failed for dsi[%d], rc = %d\n",
> + other, rc);
> + break;
> + }
> +
> + info.h_tile_instance[info.num_of_h_tiles++] = other;
> + }
> +
> + rc = dpu_encoder_setup(dev, encoder, &info);
> + if (rc)
> + DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
> + encoder->base.id, rc);
> }
>
> return rc;
> @@ -505,6 +543,7 @@ static int _dpu_kms_initialize_displayport(struct
> drm_device *dev,
> struct dpu_kms *dpu_kms)
> {
> struct drm_encoder *encoder = NULL;
> + struct msm_display_info info;
> int rc = 0;
>
> if (!priv->dp)
> @@ -516,6 +555,7 @@ static int _dpu_kms_initialize_displayport(struct
> drm_device *dev,
> return PTR_ERR(encoder);
> }
>
> + memset(&info, 0, sizeof(info));
> rc = msm_dp_modeset_init(priv->dp, dev, encoder);
> if (rc) {
> DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
> @@ -524,6 +564,14 @@ static int _dpu_kms_initialize_displayport(struct
> drm_device *dev,
> }
>
> priv->encoders[priv->num_encoders++] = encoder;
> +
> + info.num_of_h_tiles = 1;
> + info.capabilities = MSM_DISPLAY_CAP_VID_MODE;
> + info.intf_type = encoder->encoder_type;
> + rc = dpu_encoder_setup(dev, encoder, &info);
> + if (rc)
> + DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
> + encoder->base.id, rc);
> return rc;
> }
>
> @@ -726,41 +774,6 @@ static void dpu_kms_destroy(struct msm_kms *kms)
> msm_kms_destroy(&dpu_kms->base);
> }
>
> -static void _dpu_kms_set_encoder_mode(struct msm_kms *kms,
> - struct drm_encoder *encoder,
> - bool cmd_mode)
> -{
> - struct msm_display_info info;
> - struct msm_drm_private *priv = encoder->dev->dev_private;
> - int i, rc = 0;
> -
> - memset(&info, 0, sizeof(info));
> -
> - info.intf_type = encoder->encoder_type;
> - info.capabilities = cmd_mode ? MSM_DISPLAY_CAP_CMD_MODE :
> - MSM_DISPLAY_CAP_VID_MODE;
> -
> - switch (info.intf_type) {
> - case DRM_MODE_ENCODER_DSI:
> - /* TODO: No support for DSI swap */
> - for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
> - if (priv->dsi[i]) {
> - info.h_tile_instance[info.num_of_h_tiles] = i;
> - info.num_of_h_tiles++;
> - }
> - }
> - break;
> - case DRM_MODE_ENCODER_TMDS:
> - info.num_of_h_tiles = 1;
> - break;
> - }
> -
> - rc = dpu_encoder_setup(encoder->dev, encoder, &info);
> - if (rc)
> - DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
> - encoder->base.id, rc);
> -}
> -
> static irqreturn_t dpu_irq(struct msm_kms *kms)
> {
> struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> @@ -863,7 +876,6 @@ static const struct msm_kms_funcs kms_funcs = {
> .get_format = dpu_get_msm_format,
> .round_pixclk = dpu_kms_round_pixclk,
> .destroy = dpu_kms_destroy,
> - .set_encoder_mode = _dpu_kms_set_encoder_mode,
> .snapshot = dpu_kms_mdp_snapshot,
> #ifdef CONFIG_DEBUG_FS
> .debugfs_init = dpu_kms_debugfs_init,
next prev parent reply other threads:[~2021-07-12 21:15 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-10 22:19 [PATCH v3 0/7] drm/msm/dpu: add support for independent DSI config Dmitry Baryshkov
2021-07-10 22:19 ` [PATCH v3 1/7] drm/msm/dsi: rename dual DSI to bonded DSI Dmitry Baryshkov
2021-07-12 21:02 ` [Freedreno] " abhinavk
2021-07-10 22:20 ` [PATCH v3 2/7] drm/msm/dsi: add three helper functions Dmitry Baryshkov
2021-07-11 4:21 ` kernel test robot
2021-07-12 21:03 ` [Freedreno] " abhinavk
2021-07-10 22:20 ` [PATCH v3 3/7] drm/msm/dpu: support setting up two independent DSI connectors Dmitry Baryshkov
2021-07-12 21:15 ` abhinavk [this message]
2021-07-17 12:35 ` Dmitry Baryshkov
2021-07-10 22:20 ` [PATCH v3 4/7] drm/msm/mdp5: move mdp5_encoder_set_intf_mode after msm_dsi_modeset_init Dmitry Baryshkov
2021-07-10 22:20 ` [PATCH v3 5/7] drm/msm/dp: stop calling set_encoder_mode callback Dmitry Baryshkov
2021-07-10 22:20 ` [PATCH v3 6/7] drm/msm/dsi: " Dmitry Baryshkov
2021-07-10 22:20 ` [PATCH v3 7/7] drm/msm/kms: drop " Dmitry Baryshkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2b7677a4acfa1ab8ba5d67a69fea9e07@codeaurora.org \
--to=abhinavk@codeaurora.org \
--cc=airlied@linux.ie \
--cc=bjorn.andersson@linaro.org \
--cc=daniel@ffwll.ch \
--cc=dmitry.baryshkov@linaro.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=jonathan@marek.ca \
--cc=linux-arm-msm@vger.kernel.org \
--cc=robdclark@gmail.com \
--cc=sboyd@kernel.org \
--cc=sean@poorly.run \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).