On Wed, Sep 05, 2018 at 07:08:26PM -0700, Jeykumar Sankaran wrote:
> Connector states were passed around RM to update the custom
> topology connector property with chosen topology data. Now that
> we got rid of both custom properties and topology names, this
> change cleans up the mechanism to pass connector states across
> RM helpers and encoder functions.
>
> changes in v5:
> - Introduced in the series
>
> Signed-off-by: Jeykumar Sankaran <jsanka@xxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 15 +++---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 3 +-
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 3 +-
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 7 ++-
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 60 +++++-----------------
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 -
> 7 files changed, 27 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 0d43525..18f5d1d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -436,15 +436,14 @@ int dpu_encoder_helper_unregister_irq(struct dpu_encoder_phys *phys_enc,
> }
>
> void dpu_encoder_get_hw_resources(struct drm_encoder *drm_enc,
> - struct dpu_encoder_hw_resources *hw_res,
> - struct drm_connector_state *conn_state)
> + struct dpu_encoder_hw_resources *hw_res)
> {
> struct dpu_encoder_virt *dpu_enc = NULL;
> int i = 0;
>
> - if (!hw_res || !drm_enc || !conn_state) {
> - DPU_ERROR("invalid argument(s), drm_enc %d, res %d, state %d\n",
> - drm_enc != 0, hw_res != 0, conn_state != 0);
> + if (!hw_res || !drm_enc) {
> + DPU_ERROR("invalid argument(s), drm_enc %d, res %d\n",
> + drm_enc != 0, hw_res != 0);
As far as I can tell both hw_res and drm_enc will be set for all the callers so
this check shouldn't be needed. If you are paranoid you can switch to a
WARN_ON() and get rid of the custom error message but I don't think you need to.
> return;
> }
>
> @@ -458,7 +457,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder *drm_enc,
> struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>
> if (phys && phys->ops.get_hw_resources)
> - phys->ops.get_hw_resources(phys, hw_res, conn_state);
> + phys->ops.get_hw_resources(phys, hw_res);
> }
> }
>
> @@ -652,7 +651,7 @@ static int dpu_encoder_virt_atomic_check(
> if (drm_atomic_crtc_needs_modeset(crtc_state)
> && dpu_enc->mode_set_complete) {
> ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, crtc_state,
> - conn_state, topology, true);
> + topology, true);
> dpu_enc->mode_set_complete = false;
> }
> }
> @@ -1044,7 +1043,7 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>
> /* Reserve dynamic resources now. Indicating non-AtomicTest phase */
> ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, drm_enc->crtc->state,
> - conn->state, topology, false);
> + topology, false);
> if (ret) {
> DPU_ERROR_ENC(dpu_enc,
> "failed to reserve hw resources, %d\n", ret);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> index f109b4d..34ac5b6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> @@ -51,11 +51,9 @@ struct dpu_encoder_kickoff_params {
> * dpu_encoder_get_hw_resources - Populate table of required hardware resources
> * @encoder: encoder pointer
> * @hw_res: resource table to populate with encoder required resources
> - * @conn_state: report hw reqs based on this proposed connector state
> */
> void dpu_encoder_get_hw_resources(struct drm_encoder *encoder,
> - struct dpu_encoder_hw_resources *hw_res,
> - struct drm_connector_state *conn_state);
> + struct dpu_encoder_hw_resources *hw_res);
>
> /**
> * dpu_encoder_register_vblank_callback - provide callback to encoder that
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index a00b222..3fe4ed9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -140,8 +140,7 @@ struct dpu_encoder_phys_ops {
> struct drm_connector_state *conn_state);
> void (*destroy)(struct dpu_encoder_phys *encoder);
> void (*get_hw_resources)(struct dpu_encoder_phys *encoder,
> - struct dpu_encoder_hw_resources *hw_res,
> - struct drm_connector_state *conn_state);
> + struct dpu_encoder_hw_resources *hw_res);
> int (*control_vblank_irq)(struct dpu_encoder_phys *enc, bool enable);
> int (*wait_for_commit_done)(struct dpu_encoder_phys *phys_enc);
> int (*wait_for_tx_complete)(struct dpu_encoder_phys *phys_enc);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> index 5c89868..f277a69 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> @@ -599,8 +599,7 @@ static void dpu_encoder_phys_cmd_destroy(struct dpu_encoder_phys *phys_enc)
>
> static void dpu_encoder_phys_cmd_get_hw_resources(
> struct dpu_encoder_phys *phys_enc,
> - struct dpu_encoder_hw_resources *hw_res,
> - struct drm_connector_state *conn_state)
> + struct dpu_encoder_hw_resources *hw_res)
> {
> struct dpu_encoder_phys_cmd *cmd_enc =
> to_dpu_encoder_phys_cmd(phys_enc);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index 6de13f4..fd51fe6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -537,12 +537,11 @@ static void dpu_encoder_phys_vid_destroy(struct dpu_encoder_phys *phys_enc)
>
> static void dpu_encoder_phys_vid_get_hw_resources(
> struct dpu_encoder_phys *phys_enc,
> - struct dpu_encoder_hw_resources *hw_res,
> - struct drm_connector_state *conn_state)
> + struct dpu_encoder_hw_resources *hw_res)
> {
> if (!phys_enc || !hw_res) {
> - DPU_ERROR("invalid arg(s), enc %d hw_res %d conn_state %d\n",
> - phys_enc != 0, hw_res != 0, conn_state != 0);
> + DPU_ERROR("invalid arg(s), enc %d hw_res %d\n",
> + phys_enc != 0, hw_res != 0);
I don't think this is possible either.
> return;
> }
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index 86466f0..1164561 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -682,7 +682,6 @@ static int _dpu_rm_make_next_rsvp(
> struct dpu_rm *rm,
> struct drm_encoder *enc,
> struct drm_crtc_state *crtc_state,
> - struct drm_connector_state *conn_state,
> struct dpu_rm_rsvp *rsvp,
> struct dpu_rm_requirements *reqs)
> {
> @@ -728,7 +727,6 @@ static int _dpu_rm_populate_requirements(
> struct dpu_rm *rm,
> struct drm_encoder *enc,
> struct drm_crtc_state *crtc_state,
> - struct drm_connector_state *conn_state,
> struct dpu_rm_requirements *reqs,
> struct msm_display_topology req_topology)
> {
> @@ -736,7 +734,7 @@ static int _dpu_rm_populate_requirements(
>
> memset(reqs, 0, sizeof(*reqs));
>
> - dpu_encoder_get_hw_resources(enc, &reqs->hw_res, conn_state);
> + dpu_encoder_get_hw_resources(enc, &reqs->hw_res);
>
> for (i = 0; i < DPU_RM_TOPOLOGY_MAX; i++) {
> if (RM_IS_TOPOLOGY_MATCH(g_top_table[i],
> @@ -780,29 +778,12 @@ static struct dpu_rm_rsvp *_dpu_rm_get_rsvp(
> return NULL;
> }
>
> -static struct drm_connector *_dpu_rm_get_connector(
> - struct drm_encoder *enc)
> -{
> - struct drm_connector *conn = NULL;
> - struct list_head *connector_list =
> - &enc->dev->mode_config.connector_list;
> -
> - list_for_each_entry(conn, connector_list, head)
> - if (conn->encoder == enc)
> - return conn;
> -
> - return NULL;
> -}
> -
> /**
> * _dpu_rm_release_rsvp - release resources and release a reservation
> * @rm: KMS handle
> * @rsvp: RSVP pointer to release and release resources for
> */
> -static void _dpu_rm_release_rsvp(
> - struct dpu_rm *rm,
> - struct dpu_rm_rsvp *rsvp,
> - struct drm_connector *conn)
> +static void _dpu_rm_release_rsvp(struct dpu_rm *rm, struct dpu_rm_rsvp *rsvp)
> {
> struct dpu_rm_rsvp *rsvp_c, *rsvp_n;
> struct dpu_rm_hw_blk *blk;
> @@ -843,7 +824,6 @@ static void _dpu_rm_release_rsvp(
> void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc)
> {
> struct dpu_rm_rsvp *rsvp;
> - struct drm_connector *conn;
>
> if (!rm || !enc) {
> DPU_ERROR("invalid params\n");
> @@ -858,21 +838,12 @@ void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc)
> goto end;
> }
>
> - conn = _dpu_rm_get_connector(enc);
> - if (!conn) {
> - DPU_ERROR("failed to get connector for enc %d\n", enc->base.id);
> - goto end;
> - }
> -
> - _dpu_rm_release_rsvp(rm, rsvp, conn);
> + _dpu_rm_release_rsvp(rm, rsvp);
> end:
> mutex_unlock(&rm->rm_lock);
> }
>
> -static int _dpu_rm_commit_rsvp(
> - struct dpu_rm *rm,
> - struct dpu_rm_rsvp *rsvp,
> - struct drm_connector_state *conn_state)
> +static int _dpu_rm_commit_rsvp(struct dpu_rm *rm, struct dpu_rm_rsvp *rsvp)
> {
> struct dpu_rm_hw_blk *blk;
> enum dpu_hw_blk_type type;
> @@ -899,7 +870,6 @@ int dpu_rm_reserve(
> struct dpu_rm *rm,
> struct drm_encoder *enc,
> struct drm_crtc_state *crtc_state,
> - struct drm_connector_state *conn_state,
> struct msm_display_topology topology,
> bool test_only)
> {
> @@ -907,7 +877,7 @@ int dpu_rm_reserve(
> struct dpu_rm_requirements reqs;
> int ret;
>
> - if (!rm || !enc || !crtc_state || !conn_state) {
> + if (!rm || !enc || !crtc_state) {
I also don't think this is possible.
> DPU_ERROR("invalid arguments\n");
> return -EINVAL;
> }
<snip>
Jordan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
