[Nouveau] [PATCH CI 2/2] drm/connector: Allow max possible encoders to attach to a connector
William Lewis
minutemaidpark at hotmail.com
Mon Sep 16 12:46:15 UTC 2019
On 9/13/19 6:28 PM, José Roberto de Souza wrote:
> Currently we restrict the number of encoders that can be linked to
> a connector to 3, increase it to match the maximum number of encoders
> that can be initialized(32).
>
> To more effiently do that lets switch from an array of encoder ids to
> bitmask.
Typo, s/effiently/efficiently/.
>
> v2: Fixing missed return on amdgpu_dm_connector_to_encoder()
>
> Suggested-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Cc: dri-devel at lists.freedesktop.org
> Cc: intel-gfx at lists.freedesktop.org
> Cc: nouveau at lists.freedesktop.org
> Cc: amd-gfx at lists.freedesktop.org
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_connectors.c | 23 +++++---------
> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 5 ++-
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++++-
> drivers/gpu/drm/drm_client_modeset.c | 3 +-
> drivers/gpu/drm/drm_connector.c | 31 +++++--------------
> drivers/gpu/drm/drm_crtc_helper.c | 9 ++++--
> drivers/gpu/drm/drm_probe_helper.c | 3 +-
> drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +-
> drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_connector.c | 7 ++---
> drivers/gpu/drm/radeon/radeon_connectors.c | 27 ++++++----------
> include/drm/drm_connector.h | 18 +++++------
> 12 files changed, 55 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index ece55c8fa673..d8729285f731 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -217,11 +217,10 @@ amdgpu_connector_update_scratch_regs(struct drm_connector *connector,
> struct drm_encoder *encoder;
> const struct drm_connector_helper_funcs *connector_funcs = connector->helper_private;
> bool connected;
> - int i;
>
> best_encoder = connector_funcs->best_encoder(connector);
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> if ((encoder == best_encoder) && (status == connector_status_connected))
> connected = true;
> else
> @@ -236,9 +235,8 @@ amdgpu_connector_find_encoder(struct drm_connector *connector,
> int encoder_type)
> {
> struct drm_encoder *encoder;
> - int i;
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> if (encoder->encoder_type == encoder_type)
> return encoder;
> }
> @@ -347,10 +345,9 @@ static struct drm_encoder *
> amdgpu_connector_best_single_encoder(struct drm_connector *connector)
> {
> struct drm_encoder *encoder;
> - int i;
>
> /* pick the first one */
> - drm_connector_for_each_possible_encoder(connector, encoder, i)
> + drm_connector_for_each_possible_encoder(connector, encoder)
> return encoder;
>
> return NULL;
> @@ -1065,9 +1062,8 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force)
> /* find analog encoder */
> if (amdgpu_connector->dac_load_detect) {
> struct drm_encoder *encoder;
> - int i;
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> if (encoder->encoder_type != DRM_MODE_ENCODER_DAC &&
> encoder->encoder_type != DRM_MODE_ENCODER_TVDAC)
> continue;
> @@ -1117,9 +1113,8 @@ amdgpu_connector_dvi_encoder(struct drm_connector *connector)
> {
> struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector);
> struct drm_encoder *encoder;
> - int i;
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> if (amdgpu_connector->use_digital == true) {
> if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS)
> return encoder;
> @@ -1134,7 +1129,7 @@ amdgpu_connector_dvi_encoder(struct drm_connector *connector)
>
> /* then check use digitial */
> /* pick the first one */
> - drm_connector_for_each_possible_encoder(connector, encoder, i)
> + drm_connector_for_each_possible_encoder(connector, encoder)
> return encoder;
>
> return NULL;
> @@ -1271,9 +1266,8 @@ u16 amdgpu_connector_encoder_get_dp_bridge_encoder_id(struct drm_connector *conn
> {
> struct drm_encoder *encoder;
> struct amdgpu_encoder *amdgpu_encoder;
> - int i;
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> amdgpu_encoder = to_amdgpu_encoder(encoder);
>
> switch (amdgpu_encoder->encoder_id) {
> @@ -1292,10 +1286,9 @@ static bool amdgpu_connector_encoder_is_hbr2(struct drm_connector *connector)
> {
> struct drm_encoder *encoder;
> struct amdgpu_encoder *amdgpu_encoder;
> - int i;
> bool found = false;
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> amdgpu_encoder = to_amdgpu_encoder(encoder);
> if (amdgpu_encoder->caps & ATOM_ENCODER_CAP_RECORD_HBR2)
> found = true;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> index c9608ae8643b..e4f94863332c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> @@ -260,15 +260,14 @@ static struct drm_encoder *
> dce_virtual_encoder(struct drm_connector *connector)
> {
> struct drm_encoder *encoder;
> - int i;
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> if (encoder->encoder_type == DRM_MODE_ENCODER_VIRTUAL)
> return encoder;
> }
>
> /* pick the first one */
> - drm_connector_for_each_possible_encoder(connector, encoder, i)
> + drm_connector_for_each_possible_encoder(connector, encoder)
> return encoder;
>
> return NULL;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index e1b09bb432bd..860c29c489c2 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4836,7 +4836,13 @@ static int to_drm_connector_type(enum signal_type st)
>
> static struct drm_encoder *amdgpu_dm_connector_to_encoder(struct drm_connector *connector)
> {
> - return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
> + struct drm_encoder *encoder;
> +
> + /* There is only one encoder per connector */
> + drm_connector_for_each_possible_encoder(connector, encoder)
> + return encoder;
> +
> + return NULL;
> }
>
> static void amdgpu_dm_get_native_mode(struct drm_connector *connector)
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index c8922b7cac09..895b73f23079 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -415,9 +415,8 @@ static bool connector_has_possible_crtc(struct drm_connector *connector,
> struct drm_crtc *crtc)
> {
> struct drm_encoder *encoder;
> - int i;
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> if (encoder->possible_crtcs & drm_crtc_mask(crtc))
> return true;
> }
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 4c766624b20d..43896c711b50 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -365,8 +365,6 @@ EXPORT_SYMBOL(drm_connector_attach_edid_property);
> int drm_connector_attach_encoder(struct drm_connector *connector,
> struct drm_encoder *encoder)
> {
> - int i;
> -
> /*
> * In the past, drivers have attempted to model the static association
> * of connector to encoder in simple connector/encoder devices using a
> @@ -381,18 +379,15 @@ int drm_connector_attach_encoder(struct drm_connector *connector,
> if (WARN_ON(connector->encoder))
> return -EINVAL;
>
> - for (i = 0; i < ARRAY_SIZE(connector->encoder_ids); i++) {
> - if (connector->encoder_ids[i] == 0) {
> - connector->encoder_ids[i] = encoder->base.id;
> - return 0;
> - }
> - }
> - return -ENOMEM;
> + connector->possible_encoders |= drm_encoder_mask(encoder);
> +
> + return 0;
> }
> EXPORT_SYMBOL(drm_connector_attach_encoder);
>
> /**
> - * drm_connector_has_possible_encoder - check if the connector and encoder are assosicated with each other
> + * drm_connector_has_possible_encoder - check if the connector and encoder are
> + * associated with each other
> * @connector: the connector
> * @encoder: the encoder
> *
> @@ -402,15 +397,7 @@ EXPORT_SYMBOL(drm_connector_attach_encoder);
> bool drm_connector_has_possible_encoder(struct drm_connector *connector,
> struct drm_encoder *encoder)
> {
> - struct drm_encoder *enc;
> - int i;
> -
> - drm_connector_for_each_possible_encoder(connector, enc, i) {
> - if (enc == encoder)
> - return true;
> - }
> -
> - return false;
> + return connector->possible_encoders & drm_encoder_mask(encoder);
> }
> EXPORT_SYMBOL(drm_connector_has_possible_encoder);
>
> @@ -2121,7 +2108,6 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> int encoders_count = 0;
> int ret = 0;
> int copied = 0;
> - int i;
> struct drm_mode_modeinfo u_mode;
> struct drm_mode_modeinfo __user *mode_ptr;
> uint32_t __user *encoder_ptr;
> @@ -2136,14 +2122,13 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> if (!connector)
> return -ENOENT;
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i)
> - encoders_count++;
> + encoders_count = hweight32(connector->possible_encoders);
>
> if ((out_resp->count_encoders >= encoders_count) && encoders_count) {
> copied = 0;
> encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr);
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> if (put_user(encoder->base.id, encoder_ptr + copied)) {
> ret = -EFAULT;
> goto out;
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 4a7447a53cea..499b05aaccfc 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -467,8 +467,13 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
> struct drm_encoder *
> drm_connector_get_single_encoder(struct drm_connector *connector)
> {
> - WARN_ON(connector->encoder_ids[1]);
> - return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
> + struct drm_encoder *encoder;
> +
> + WARN_ON(hweight32(connector->possible_encoders) > 1);
> + drm_connector_for_each_possible_encoder(connector, encoder)
> + return encoder;
> +
> + return NULL;
> }
>
> /**
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 351cbc40f0f8..a7c87abe88d0 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -93,7 +93,6 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
> struct drm_device *dev = connector->dev;
> enum drm_mode_status ret = MODE_OK;
> struct drm_encoder *encoder;
> - int i;
>
> /* Step 1: Validate against connector */
> ret = drm_connector_mode_valid(connector, mode);
> @@ -101,7 +100,7 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
> return ret;
>
> /* Step 2: Validate against encoders and crtcs */
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> struct drm_crtc *crtc;
>
> ret = drm_encoder_mode_valid(encoder, mode);
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> index dc64863b5fd8..44ee82d0c9b6 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> @@ -256,7 +256,7 @@ nv04_display_create(struct drm_device *dev)
>
> list_for_each_entry_safe(connector, ct,
> &dev->mode_config.connector_list, head) {
> - if (!connector->encoder_ids[0]) {
> + if (!connector->possible_encoders) {
> NV_WARN(drm, "%s has no encoders, removing\n",
> connector->name);
> connector->funcs->destroy(connector);
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 307584107d77..98c87195d711 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -2391,7 +2391,7 @@ nv50_display_create(struct drm_device *dev)
>
> /* cull any connectors we created that don't have an encoder */
> list_for_each_entry_safe(connector, tmp, &dev->mode_config.connector_list, head) {
> - if (connector->encoder_ids[0])
> + if (connector->possible_encoders)
> continue;
>
> NV_WARN(drm, "%s has no encoders, removing\n",
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 56871d34e3fb..3a5db17bc5c7 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -365,9 +365,8 @@ find_encoder(struct drm_connector *connector, int type)
> {
> struct nouveau_encoder *nv_encoder;
> struct drm_encoder *enc;
> - int i;
>
> - drm_connector_for_each_possible_encoder(connector, enc, i) {
> + drm_connector_for_each_possible_encoder(connector, enc) {
> nv_encoder = nouveau_encoder(enc);
>
> if (type == DCB_OUTPUT_ANY ||
> @@ -414,10 +413,10 @@ nouveau_connector_ddc_detect(struct drm_connector *connector)
> struct drm_device *dev = connector->dev;
> struct nouveau_encoder *nv_encoder = NULL, *found = NULL;
> struct drm_encoder *encoder;
> - int i, ret;
> + int ret;
> bool switcheroo_ddc = false;
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> nv_encoder = nouveau_encoder(encoder);
>
> switch (nv_encoder->dcb->type) {
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
> index b684cd719612..c07427d3c199 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -249,11 +249,10 @@ radeon_connector_update_scratch_regs(struct drm_connector *connector, enum drm_c
> struct drm_encoder *encoder;
> const struct drm_connector_helper_funcs *connector_funcs = connector->helper_private;
> bool connected;
> - int i;
>
> best_encoder = connector_funcs->best_encoder(connector);
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> if ((encoder == best_encoder) && (status == connector_status_connected))
> connected = true;
> else
> @@ -269,9 +268,8 @@ radeon_connector_update_scratch_regs(struct drm_connector *connector, enum drm_c
> static struct drm_encoder *radeon_find_encoder(struct drm_connector *connector, int encoder_type)
> {
> struct drm_encoder *encoder;
> - int i;
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> if (encoder->encoder_type == encoder_type)
> return encoder;
> }
> @@ -380,10 +378,9 @@ static int radeon_ddc_get_modes(struct drm_connector *connector)
> static struct drm_encoder *radeon_best_single_encoder(struct drm_connector *connector)
> {
> struct drm_encoder *encoder;
> - int i;
>
> /* pick the first one */
> - drm_connector_for_each_possible_encoder(connector, encoder, i)
> + drm_connector_for_each_possible_encoder(connector, encoder)
> return encoder;
>
> return NULL;
> @@ -428,14 +425,13 @@ radeon_connector_analog_encoder_conflict_solve(struct drm_connector *connector,
>
> list_for_each_entry(conflict, &dev->mode_config.connector_list, head) {
> struct drm_encoder *enc;
> - int i;
>
> if (conflict == connector)
> continue;
>
> radeon_conflict = to_radeon_connector(conflict);
>
> - drm_connector_for_each_possible_encoder(conflict, enc, i) {
> + drm_connector_for_each_possible_encoder(conflict, enc) {
> /* if the IDs match */
> if (enc == encoder) {
> if (conflict->status != connector_status_connected)
> @@ -1363,9 +1359,7 @@ radeon_dvi_detect(struct drm_connector *connector, bool force)
>
> /* find analog encoder */
> if (radeon_connector->dac_load_detect) {
> - int i;
> -
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> if (encoder->encoder_type != DRM_MODE_ENCODER_DAC &&
> encoder->encoder_type != DRM_MODE_ENCODER_TVDAC)
> continue;
> @@ -1443,9 +1437,8 @@ static struct drm_encoder *radeon_dvi_encoder(struct drm_connector *connector)
> {
> struct radeon_connector *radeon_connector = to_radeon_connector(connector);
> struct drm_encoder *encoder;
> - int i;
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> if (radeon_connector->use_digital == true) {
> if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS)
> return encoder;
> @@ -1460,7 +1453,7 @@ static struct drm_encoder *radeon_dvi_encoder(struct drm_connector *connector)
>
> /* then check use digitial */
> /* pick the first one */
> - drm_connector_for_each_possible_encoder(connector, encoder, i)
> + drm_connector_for_each_possible_encoder(connector, encoder)
> return encoder;
>
> return NULL;
> @@ -1603,9 +1596,8 @@ u16 radeon_connector_encoder_get_dp_bridge_encoder_id(struct drm_connector *conn
> {
> struct drm_encoder *encoder;
> struct radeon_encoder *radeon_encoder;
> - int i;
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> radeon_encoder = to_radeon_encoder(encoder);
>
> switch (radeon_encoder->encoder_id) {
> @@ -1624,10 +1616,9 @@ static bool radeon_connector_encoder_is_hbr2(struct drm_connector *connector)
> {
> struct drm_encoder *encoder;
> struct radeon_encoder *radeon_encoder;
> - int i;
> bool found = false;
>
> - drm_connector_for_each_possible_encoder(connector, encoder, i) {
> + drm_connector_for_each_possible_encoder(connector, encoder) {
> radeon_encoder = to_radeon_encoder(encoder);
> if (radeon_encoder->caps & ATOM_ENCODER_CAP_RECORD_HBR2)
> found = true;
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 681cb590f952..c6e993e78dbd 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1288,12 +1288,12 @@ struct drm_connector {
> /** @override_edid: has the EDID been overwritten through debugfs for testing? */
> bool override_edid;
>
> -#define DRM_CONNECTOR_MAX_ENCODER 3
> /**
> - * @encoder_ids: Valid encoders for this connector. Please only use
> - * drm_connector_for_each_possible_encoder() to enumerate these.
> + * @possible_encoders: Bit mask of encoders that can drive this
> + * connector, drm_encoder_index() determines the index into the bitfield
> + * and the bits are set with drm_connector_attach_encoder().
> */
> - uint32_t encoder_ids[DRM_CONNECTOR_MAX_ENCODER];
> + u32 possible_encoders;
>
> /**
> * @encoder: Currently bound encoder driving this connector, if any.
> @@ -1608,13 +1608,9 @@ bool drm_connector_has_possible_encoder(struct drm_connector *connector,
> * drm_connector_for_each_possible_encoder - iterate connector's possible encoders
> * @connector: &struct drm_connector pointer
> * @encoder: &struct drm_encoder pointer used as cursor
> - * @__i: int iteration cursor, for macro-internal use
> */
> -#define drm_connector_for_each_possible_encoder(connector, encoder, __i) \
> - for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
> - (connector)->encoder_ids[(__i)] != 0; (__i)++) \
> - for_each_if((encoder) = \
> - drm_encoder_find((connector)->dev, NULL, \
> - (connector)->encoder_ids[(__i)])) \
> +#define drm_connector_for_each_possible_encoder(connector, encoder) \
> + drm_for_each_encoder_mask(encoder, (connector)->dev, \
> + (connector)->possible_encoders)
>
> #endif
More information about the Nouveau
mailing list