From: Ville Syrjälä ville.syrjala@linux.intel.com
Changes from the previous version mainly involve Danoie's suggestion of hiding the drm_encoder_find() in the iterator macro. I also polished the msm and tilcdc cases a bit more with another small helper.
Cc: Alex Deucher alexander.deucher@amd.com Cc: amd-gfx@lists.freedesktop.org Cc: Ben Skeggs bskeggs@redhat.com Cc: "Christian König" christian.koenig@amd.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: "David (ChunMing) Zhou" David1.Zhou@amd.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: freedreno@lists.freedesktop.org Cc: Harry Wentland harry.wentland@amd.com Cc: Jyri Sarha jsarha@ti.com Cc: linux-arm-msm@vger.kernel.org Cc: nouveau@lists.freedesktop.org Cc: Rob Clark robdclark@gmail.com Cc: Tomi Valkeinen tomi.valkeinen@ti.com
Ville Syrjälä (9): drm/fb-helper: Eliminate the .best_encoder() usage drm/i915: Nuke intel_mst_best_encoder() drm: Add drm_connector_for_each_possible_encoder() drm/amdgpu: Use drm_connector_for_each_possible_encoder() drm/nouveau: Use drm_connector_for_each_possible_encoder() drm/radeon: Use drm_connector_for_each_possible_encoder() drm: Add drm_connector_has_possible_encoder() drm/msm: Use drm_connector_has_possible_encoder() drm/tilcdc: Use drm_connector_has_possible_encoder()
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 81 ++++++----------------- drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 15 ++--- drivers/gpu/drm/drm_connector.c | 44 +++++++++---- drivers/gpu/drm/drm_fb_helper.c | 34 +++++----- drivers/gpu/drm/drm_probe_helper.c | 10 +-- drivers/gpu/drm/i915/intel_dp_mst.c | 10 --- drivers/gpu/drm/msm/dsi/dsi_manager.c | 8 +-- drivers/gpu/drm/nouveau/nouveau_connector.c | 21 +----- drivers/gpu/drm/radeon/radeon_connectors.c | 90 ++++++++------------------ drivers/gpu/drm/tilcdc/tilcdc_external.c | 9 ++- include/drm/drm_connector.h | 16 +++++ 11 files changed, 128 insertions(+), 210 deletions(-)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of using the .best_encoder() hook to figure out whether a given connector+crtc combo will work, let's instead do what userspace does and just iterate over all the encoders for the connector, and then check each crtc against each encoder's possible_crtcs bitmask.
v2: Avoid oopsing on NULL encoders (Daniel) s/connector_crtc_ok/connector_has_possible_crtc/
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Reviewed-by: Alex Deucher alexander.deucher@amd.com #v1 Suggested-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_fb_helper.c | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index cab14f253384..b37f06317d51 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2323,6 +2323,27 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper, return true; }
+static bool connector_has_possible_crtc(struct drm_connector *connector, + struct drm_crtc *crtc) +{ + int i; + + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { + struct drm_encoder *encoder; + + if (connector->encoder_ids[i] == 0) + break; + + encoder = drm_encoder_find(connector->dev, NULL, + connector->encoder_ids[i]); + + if (encoder->possible_crtcs & drm_crtc_mask(crtc)) + return true; + } + + return false; +} + static int drm_pick_crtcs(struct drm_fb_helper *fb_helper, struct drm_fb_helper_crtc **best_crtcs, struct drm_display_mode **modes, @@ -2331,7 +2352,6 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper, int c, o; struct drm_connector *connector; const struct drm_connector_helper_funcs *connector_funcs; - struct drm_encoder *encoder; int my_score, best_score, score; struct drm_fb_helper_crtc **crtcs, *crtc; struct drm_fb_helper_connector *fb_helper_conn; @@ -2362,20 +2382,6 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
connector_funcs = connector->helper_private;
- /* - * If the DRM device implements atomic hooks and ->best_encoder() is - * NULL we fallback to the default drm_atomic_helper_best_encoder() - * helper. - */ - if (drm_drv_uses_atomic_modeset(fb_helper->dev) && - !connector_funcs->best_encoder) - encoder = drm_atomic_helper_best_encoder(connector); - else - encoder = connector_funcs->best_encoder(connector); - - if (!encoder) - goto out; - /* * select a crtc for this connector and then attempt to configure * remaining connectors @@ -2383,7 +2389,8 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper, for (c = 0; c < fb_helper->crtc_count; c++) { crtc = &fb_helper->crtc_info[c];
- if ((encoder->possible_crtcs & (1 << c)) == 0) + if (!connector_has_possible_crtc(connector, + crtc->mode_set.crtc)) continue;
for (o = 0; o < n; o++) @@ -2410,7 +2417,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper, sizeof(struct drm_fb_helper_crtc *)); } } -out: + kfree(crtcs); return best_score; }
On Thu, Jun 28, 2018 at 9:13 AM, Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of using the .best_encoder() hook to figure out whether a given connector+crtc combo will work, let's instead do what userspace does and just iterate over all the encoders for the connector, and then check each crtc against each encoder's possible_crtcs bitmask.
v2: Avoid oopsing on NULL encoders (Daniel) s/connector_crtc_ok/connector_has_possible_crtc/
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Reviewed-by: Alex Deucher alexander.deucher@amd.com #v1 Suggested-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Series is: Reviewed-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/drm_fb_helper.c | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index cab14f253384..b37f06317d51 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2323,6 +2323,27 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper, return true; }
+static bool connector_has_possible_crtc(struct drm_connector *connector,
struct drm_crtc *crtc)+{
int i;for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {struct drm_encoder *encoder;if (connector->encoder_ids[i] == 0)break;encoder = drm_encoder_find(connector->dev, NULL,connector->encoder_ids[i]);if (encoder->possible_crtcs & drm_crtc_mask(crtc))return true;}return false;+}
static int drm_pick_crtcs(struct drm_fb_helper *fb_helper, struct drm_fb_helper_crtc **best_crtcs, struct drm_display_mode **modes, @@ -2331,7 +2352,6 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper, int c, o; struct drm_connector *connector; const struct drm_connector_helper_funcs *connector_funcs;
struct drm_encoder *encoder; int my_score, best_score, score; struct drm_fb_helper_crtc **crtcs, *crtc; struct drm_fb_helper_connector *fb_helper_conn;@@ -2362,20 +2382,6 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
connector_funcs = connector->helper_private;
/** If the DRM device implements atomic hooks and ->best_encoder() is* NULL we fallback to the default drm_atomic_helper_best_encoder()* helper.*/if (drm_drv_uses_atomic_modeset(fb_helper->dev) &&!connector_funcs->best_encoder)encoder = drm_atomic_helper_best_encoder(connector);elseencoder = connector_funcs->best_encoder(connector);if (!encoder)goto out;/* * select a crtc for this connector and then attempt to configure * remaining connectors@@ -2383,7 +2389,8 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper, for (c = 0; c < fb_helper->crtc_count; c++) { crtc = &fb_helper->crtc_info[c];
if ((encoder->possible_crtcs & (1 << c)) == 0)
if (!connector_has_possible_crtc(connector,crtc->mode_set.crtc)) continue; for (o = 0; o < n; o++)@@ -2410,7 +2417,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper, sizeof(struct drm_fb_helper_crtc *)); } } -out:
kfree(crtcs); return best_score;}
2.16.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jun 28, 2018 at 11:27:07AM -0400, Alex Deucher wrote:
On Thu, Jun 28, 2018 at 9:13 AM, Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of using the .best_encoder() hook to figure out whether a given connector+crtc combo will work, let's instead do what userspace does and just iterate over all the encoders for the connector, and then check each crtc against each encoder's possible_crtcs bitmask.
v2: Avoid oopsing on NULL encoders (Daniel) s/connector_crtc_ok/connector_has_possible_crtc/
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Harry Wentland harry.wentland@amd.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Reviewed-by: Alex Deucher alexander.deucher@amd.com #v1 Suggested-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Series is: Reviewed-by: Alex Deucher alexander.deucher@amd.com
Thanks for the review. Series pushed to drm-misc-next.
drivers/gpu/drm/drm_fb_helper.c | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index cab14f253384..b37f06317d51 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2323,6 +2323,27 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper, return true; }
+static bool connector_has_possible_crtc(struct drm_connector *connector,
struct drm_crtc *crtc)+{
int i;for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {struct drm_encoder *encoder;if (connector->encoder_ids[i] == 0)break;encoder = drm_encoder_find(connector->dev, NULL,connector->encoder_ids[i]);if (encoder->possible_crtcs & drm_crtc_mask(crtc))return true;}return false;+}
static int drm_pick_crtcs(struct drm_fb_helper *fb_helper, struct drm_fb_helper_crtc **best_crtcs, struct drm_display_mode **modes, @@ -2331,7 +2352,6 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper, int c, o; struct drm_connector *connector; const struct drm_connector_helper_funcs *connector_funcs;
struct drm_encoder *encoder; int my_score, best_score, score; struct drm_fb_helper_crtc **crtcs, *crtc; struct drm_fb_helper_connector *fb_helper_conn;@@ -2362,20 +2382,6 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
connector_funcs = connector->helper_private;
/** If the DRM device implements atomic hooks and ->best_encoder() is* NULL we fallback to the default drm_atomic_helper_best_encoder()* helper.*/if (drm_drv_uses_atomic_modeset(fb_helper->dev) &&!connector_funcs->best_encoder)encoder = drm_atomic_helper_best_encoder(connector);elseencoder = connector_funcs->best_encoder(connector);if (!encoder)goto out;/* * select a crtc for this connector and then attempt to configure * remaining connectors@@ -2383,7 +2389,8 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper, for (c = 0; c < fb_helper->crtc_count; c++) { crtc = &fb_helper->crtc_info[c];
if ((encoder->possible_crtcs & (1 << c)) == 0)
if (!connector_has_possible_crtc(connector,crtc->mode_set.crtc)) continue; for (o = 0; o < n; o++)@@ -2410,7 +2417,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper, sizeof(struct drm_fb_helper_crtc *)); } } -out:
kfree(crtcs); return best_score;}
2.16.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
From: Ville Syrjälä ville.syrjala@linux.intel.com
With the fb-helper no longer relying on the non-atomic .best_encoder() we can eliminate the hook from the MST encoder.
Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Reviewed-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_dp_mst.c | 10 ---------- 1 file changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 5890500a3a8b..0f012fbe34eb 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -403,20 +403,10 @@ static struct drm_encoder *intel_mst_atomic_best_encoder(struct drm_connector *c return &intel_dp->mst_encoders[crtc->pipe]->base.base; }
-static struct drm_encoder *intel_mst_best_encoder(struct drm_connector *connector) -{ - struct intel_connector *intel_connector = to_intel_connector(connector); - struct intel_dp *intel_dp = intel_connector->mst_port; - if (!intel_dp) - return NULL; - return &intel_dp->mst_encoders[0]->base.base; -} - static const struct drm_connector_helper_funcs intel_dp_mst_connector_helper_funcs = { .get_modes = intel_dp_mst_get_modes, .mode_valid = intel_dp_mst_mode_valid, .atomic_best_encoder = intel_mst_atomic_best_encoder, - .best_encoder = intel_mst_best_encoder, .atomic_check = intel_dp_mst_atomic_check, };
From: Ville Syrjälä ville.syrjala@linux.intel.com
Add a convenience macro for iterating connector->encoder_ids[]. Isolates the users from the implementation details.
Note that we don't seem to pass the file_priv down to drm_encoder_find() because encoders apparently don't get leased. No idea why drm_encoder_finc() even takes the file_priv actually.
Also use ARRAY_SIZE() when populating the array to avoid spreading knowledge about the array size all over.
v2: Hide the drm_encoder_find() in the macro, and rename the macro appropriately (Daniel)
Cc: Daniel Vetter daniel.vetter@ffwll.ch Reviewed-by: Alex Deucher alexander.deucher@amd.com #v1 Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_connector.c | 21 +++++++++------------ drivers/gpu/drm/drm_fb_helper.c | 11 ++--------- drivers/gpu/drm/drm_probe_helper.c | 10 +++------- include/drm/drm_connector.h | 13 +++++++++++++ 4 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 2f9ebddd178e..186febcf4e55 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -321,7 +321,7 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector, if (WARN_ON(connector->encoder)) return -EINVAL;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { + 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; @@ -1708,22 +1708,19 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, if (!connector) return -ENOENT;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) - if (connector->encoder_ids[i] != 0) - encoders_count++; + drm_connector_for_each_possible_encoder(connector, encoder, i) + encoders_count++;
if ((out_resp->count_encoders >= encoders_count) && encoders_count) { copied = 0; encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr); - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] != 0) { - if (put_user(connector->encoder_ids[i], - encoder_ptr + copied)) { - ret = -EFAULT; - goto out; - } - copied++; + + drm_connector_for_each_possible_encoder(connector, encoder, i) { + if (put_user(encoder->base.id, encoder_ptr + copied)) { + ret = -EFAULT; + goto out; } + copied++; } } out_resp->count_encoders = encoders_count; diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index b37f06317d51..d697c1c4a067 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2326,17 +2326,10 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper, static bool connector_has_possible_crtc(struct drm_connector *connector, struct drm_crtc *crtc) { + struct drm_encoder *encoder; int i;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - struct drm_encoder *encoder; - - if (connector->encoder_ids[i] == 0) - break; - - encoder = drm_encoder_find(connector->dev, NULL, - connector->encoder_ids[i]); - + drm_connector_for_each_possible_encoder(connector, encoder, i) { if (encoder->possible_crtcs & drm_crtc_mask(crtc)) return true; } diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 527743394150..1a901fe9e23e 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -88,9 +88,9 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode, struct drm_connector *connector) { struct drm_device *dev = connector->dev; - uint32_t *ids = connector->encoder_ids; enum drm_mode_status ret = MODE_OK; - unsigned int i; + struct drm_encoder *encoder; + int i;
/* Step 1: Validate against connector */ ret = drm_connector_mode_valid(connector, mode); @@ -98,13 +98,9 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode, return ret;
/* Step 2: Validate against encoders and crtcs */ - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - struct drm_encoder *encoder = drm_encoder_find(dev, NULL, ids[i]); + drm_connector_for_each_possible_encoder(connector, encoder, i) { struct drm_crtc *crtc;
- if (!encoder) - continue; - ret = drm_encoder_mode_valid(encoder, mode); if (ret != MODE_OK) { /* No point in continuing for crtc check as this encoder diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 14ab58ade87f..8de3a3aa516a 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1193,4 +1193,17 @@ void drm_connector_list_iter_end(struct drm_connector_list_iter *iter); #define drm_for_each_connector_iter(connector, iter) \ while ((connector = drm_connector_list_iter_next(iter)))
+/** + * drm_connector_for_each_possible_encoder - iterate connector's possible encoders + * @connector: &struct drm_connector pointer used as cursor + * @encoder: the encoder + * @__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)])) \ + #endif
On Thu, Jun 28, 2018 at 04:13:09PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Add a convenience macro for iterating connector->encoder_ids[]. Isolates the users from the implementation details.
Note that we don't seem to pass the file_priv down to drm_encoder_find() because encoders apparently don't get leased. No idea why drm_encoder_finc() even takes the file_priv actually.
Also use ARRAY_SIZE() when populating the array to avoid spreading knowledge about the array size all over.
v2: Hide the drm_encoder_find() in the macro, and rename the macro appropriately (Daniel)
Cc: Daniel Vetter daniel.vetter@ffwll.ch Reviewed-by: Alex Deucher alexander.deucher@amd.com #v1 Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_connector.c | 21 +++++++++------------ drivers/gpu/drm/drm_fb_helper.c | 11 ++--------- drivers/gpu/drm/drm_probe_helper.c | 10 +++------- include/drm/drm_connector.h | 13 +++++++++++++ 4 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 2f9ebddd178e..186febcf4e55 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -321,7 +321,7 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector, if (WARN_ON(connector->encoder)) return -EINVAL;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
- 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;
@@ -1708,22 +1708,19 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, if (!connector) return -ENOENT;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
if (connector->encoder_ids[i] != 0)encoders_count++;
drm_connector_for_each_possible_encoder(connector, encoder, i)
encoders_count++;if ((out_resp->count_encoders >= encoders_count) && encoders_count) { copied = 0; encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr);
for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {if (connector->encoder_ids[i] != 0) {if (put_user(connector->encoder_ids[i],encoder_ptr + copied)) {ret = -EFAULT;goto out;}copied++;
drm_connector_for_each_possible_encoder(connector, encoder, i) {if (put_user(encoder->base.id, encoder_ptr + copied)) {ret = -EFAULT;goto out; }copied++;diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index b37f06317d51..d697c1c4a067 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2326,17 +2326,10 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper, static bool connector_has_possible_crtc(struct drm_connector *connector, struct drm_crtc *crtc) {
- struct drm_encoder *encoder; int i;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
struct drm_encoder *encoder;if (connector->encoder_ids[i] == 0)break;encoder = drm_encoder_find(connector->dev, NULL,connector->encoder_ids[i]);
- drm_connector_for_each_possible_encoder(connector, encoder, i) { if (encoder->possible_crtcs & drm_crtc_mask(crtc)) return true; }
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 527743394150..1a901fe9e23e 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -88,9 +88,9 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode, struct drm_connector *connector) { struct drm_device *dev = connector->dev;
- uint32_t *ids = connector->encoder_ids; enum drm_mode_status ret = MODE_OK;
- unsigned int i;
struct drm_encoder *encoder;
int i;
/* Step 1: Validate against connector */ ret = drm_connector_mode_valid(connector, mode);
@@ -98,13 +98,9 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode, return ret;
/* Step 2: Validate against encoders and crtcs */
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
struct drm_encoder *encoder = drm_encoder_find(dev, NULL, ids[i]);
- drm_connector_for_each_possible_encoder(connector, encoder, i) { struct drm_crtc *crtc;
if (!encoder)continue;- ret = drm_encoder_mode_valid(encoder, mode); if (ret != MODE_OK) { /* No point in continuing for crtc check as this encoder
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 14ab58ade87f..8de3a3aa516a 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1193,4 +1193,17 @@ void drm_connector_list_iter_end(struct drm_connector_list_iter *iter); #define drm_for_each_connector_iter(connector, iter) \ while ((connector = drm_connector_list_iter_next(iter)))
+/**
- drm_connector_for_each_possible_encoder - iterate connector's possible encoders
- @connector: &struct drm_connector pointer used as cursor
- @encoder: the encoder
Isn't this the other way round? encoder is the cursor, connector is _the_ connector?
Otherwise Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch and ack from me on the entire series. Much prettier! -Daniel
- @__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)])) \#endif
2.16.4
On Thu, Jun 28, 2018 at 06:56:40PM +0200, Daniel Vetter wrote:
On Thu, Jun 28, 2018 at 04:13:09PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Add a convenience macro for iterating connector->encoder_ids[]. Isolates the users from the implementation details.
Note that we don't seem to pass the file_priv down to drm_encoder_find() because encoders apparently don't get leased. No idea why drm_encoder_finc() even takes the file_priv actually.
Also use ARRAY_SIZE() when populating the array to avoid spreading knowledge about the array size all over.
v2: Hide the drm_encoder_find() in the macro, and rename the macro appropriately (Daniel)
Cc: Daniel Vetter daniel.vetter@ffwll.ch Reviewed-by: Alex Deucher alexander.deucher@amd.com #v1 Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_connector.c | 21 +++++++++------------ drivers/gpu/drm/drm_fb_helper.c | 11 ++--------- drivers/gpu/drm/drm_probe_helper.c | 10 +++------- include/drm/drm_connector.h | 13 +++++++++++++ 4 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 2f9ebddd178e..186febcf4e55 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -321,7 +321,7 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector, if (WARN_ON(connector->encoder)) return -EINVAL;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
- 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;
@@ -1708,22 +1708,19 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, if (!connector) return -ENOENT;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
if (connector->encoder_ids[i] != 0)encoders_count++;
drm_connector_for_each_possible_encoder(connector, encoder, i)
encoders_count++;if ((out_resp->count_encoders >= encoders_count) && encoders_count) { copied = 0; encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr);
for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {if (connector->encoder_ids[i] != 0) {if (put_user(connector->encoder_ids[i],encoder_ptr + copied)) {ret = -EFAULT;goto out;}copied++;
drm_connector_for_each_possible_encoder(connector, encoder, i) {if (put_user(encoder->base.id, encoder_ptr + copied)) {ret = -EFAULT;goto out; }copied++;diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index b37f06317d51..d697c1c4a067 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2326,17 +2326,10 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper, static bool connector_has_possible_crtc(struct drm_connector *connector, struct drm_crtc *crtc) {
- struct drm_encoder *encoder; int i;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
struct drm_encoder *encoder;if (connector->encoder_ids[i] == 0)break;encoder = drm_encoder_find(connector->dev, NULL,connector->encoder_ids[i]);
- drm_connector_for_each_possible_encoder(connector, encoder, i) { if (encoder->possible_crtcs & drm_crtc_mask(crtc)) return true; }
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 527743394150..1a901fe9e23e 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -88,9 +88,9 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode, struct drm_connector *connector) { struct drm_device *dev = connector->dev;
- uint32_t *ids = connector->encoder_ids; enum drm_mode_status ret = MODE_OK;
- unsigned int i;
struct drm_encoder *encoder;
int i;
/* Step 1: Validate against connector */ ret = drm_connector_mode_valid(connector, mode);
@@ -98,13 +98,9 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode, return ret;
/* Step 2: Validate against encoders and crtcs */
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
struct drm_encoder *encoder = drm_encoder_find(dev, NULL, ids[i]);
- drm_connector_for_each_possible_encoder(connector, encoder, i) { struct drm_crtc *crtc;
if (!encoder)continue;- ret = drm_encoder_mode_valid(encoder, mode); if (ret != MODE_OK) { /* No point in continuing for crtc check as this encoder
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 14ab58ade87f..8de3a3aa516a 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1193,4 +1193,17 @@ void drm_connector_list_iter_end(struct drm_connector_list_iter *iter); #define drm_for_each_connector_iter(connector, iter) \ while ((connector = drm_connector_list_iter_next(iter)))
+/**
- drm_connector_for_each_possible_encoder - iterate connector's possible encoders
- @connector: &struct drm_connector pointer used as cursor
- @encoder: the encoder
Isn't this the other way round? encoder is the cursor, connector is _the_ connector?
Yes indeed. Copy-paste fail on my part. Will fix.
Otherwise Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch and ack from me on the entire series. Much prettier! -Daniel
- @__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)])) \#endif
2.16.4
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
From: Ville Syrjälä ville.syrjala@linux.intel.com
Use drm_connector_for_each_possible_encoder() for iterating connector->encoder_ids[]. A bit more convenient not having to deal with the implementation details.
v2: Replace drm_for_each_connector_encoder_ids() with drm_connector_for_each_possible_encoder() (Daniel)
Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Alex Deucher alexander.deucher@amd.com Cc: "Christian König" christian.koenig@amd.com Cc: "David (ChunMing) Zhou" David1.Zhou@amd.com Cc: Harry Wentland harry.wentland@amd.com Cc: amd-gfx@lists.freedesktop.org Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Alex Deucher alexander.deucher@amd.com #v1 --- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 81 +++++++------------------- drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 15 ++--- 2 files changed, 25 insertions(+), 71 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index 8e66851eb427..881f7cb7ae6e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -212,30 +212,21 @@ static void amdgpu_connector_update_scratch_regs(struct drm_connector *connector, enum drm_connector_status status) { - struct drm_encoder *best_encoder = NULL; - struct drm_encoder *encoder = NULL; + struct drm_encoder *best_encoder; + 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);
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] == 0) - break; - - encoder = drm_encoder_find(connector->dev, NULL, - connector->encoder_ids[i]); - if (!encoder) - continue; - + drm_connector_for_each_possible_encoder(connector, encoder, i) { if ((encoder == best_encoder) && (status == connector_status_connected)) connected = true; else connected = false;
amdgpu_atombios_encoder_set_bios_scratch_regs(connector, encoder, connected); - } }
@@ -246,17 +237,11 @@ amdgpu_connector_find_encoder(struct drm_connector *connector, struct drm_encoder *encoder; int i;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] == 0) - break; - encoder = drm_encoder_find(connector->dev, NULL, - connector->encoder_ids[i]); - if (!encoder) - continue; - + drm_connector_for_each_possible_encoder(connector, encoder, i) { if (encoder->encoder_type == encoder_type) return encoder; } + return NULL; }
@@ -360,11 +345,13 @@ static int amdgpu_connector_ddc_get_modes(struct drm_connector *connector) static struct drm_encoder * amdgpu_connector_best_single_encoder(struct drm_connector *connector) { - int enc_id = connector->encoder_ids[0]; + struct drm_encoder *encoder; + int i; + + /* pick the first one */ + drm_connector_for_each_possible_encoder(connector, encoder, i) + return encoder;
- /* pick the encoder ids */ - if (enc_id) - return drm_encoder_find(connector->dev, NULL, enc_id); return NULL; }
@@ -985,9 +972,8 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force) struct drm_device *dev = connector->dev; struct amdgpu_device *adev = dev->dev_private; struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector); - struct drm_encoder *encoder = NULL; const struct drm_encoder_helper_funcs *encoder_funcs; - int i, r; + int r; enum drm_connector_status ret = connector_status_disconnected; bool dret = false, broken_edid = false;
@@ -1077,14 +1063,10 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force)
/* find analog encoder */ if (amdgpu_connector->dac_load_detect) { - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] == 0) - break; - - encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); - if (!encoder) - continue; + struct drm_encoder *encoder; + int i;
+ drm_connector_for_each_possible_encoder(connector, encoder, i) { if (encoder->encoder_type != DRM_MODE_ENCODER_DAC && encoder->encoder_type != DRM_MODE_ENCODER_TVDAC) continue; @@ -1132,18 +1114,11 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force) static struct drm_encoder * amdgpu_connector_dvi_encoder(struct drm_connector *connector) { - int enc_id = connector->encoder_ids[0]; struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector); struct drm_encoder *encoder; int i; - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] == 0) - break; - - encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); - if (!encoder) - continue;
+ drm_connector_for_each_possible_encoder(connector, encoder, i) { if (amdgpu_connector->use_digital == true) { if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS) return encoder; @@ -1158,8 +1133,9 @@ amdgpu_connector_dvi_encoder(struct drm_connector *connector)
/* then check use digitial */ /* pick the first one */ - if (enc_id) - return drm_encoder_find(connector->dev, NULL, enc_id); + drm_connector_for_each_possible_encoder(connector, encoder, i) + return encoder; + return NULL; }
@@ -1296,15 +1272,7 @@ u16 amdgpu_connector_encoder_get_dp_bridge_encoder_id(struct drm_connector *conn struct amdgpu_encoder *amdgpu_encoder; int i;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] == 0) - break; - - encoder = drm_encoder_find(connector->dev, NULL, - connector->encoder_ids[i]); - if (!encoder) - continue; - + drm_connector_for_each_possible_encoder(connector, encoder, i) { amdgpu_encoder = to_amdgpu_encoder(encoder);
switch (amdgpu_encoder->encoder_id) { @@ -1326,14 +1294,7 @@ static bool amdgpu_connector_encoder_is_hbr2(struct drm_connector *connector) int i; bool found = false;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] == 0) - break; - encoder = drm_encoder_find(connector->dev, NULL, - connector->encoder_ids[i]); - if (!encoder) - continue; - + drm_connector_for_each_possible_encoder(connector, encoder, i) { 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 dbf2ccd0c744..016f15093173 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c @@ -269,25 +269,18 @@ static int dce_virtual_early_init(void *handle) static struct drm_encoder * dce_virtual_encoder(struct drm_connector *connector) { - int enc_id = connector->encoder_ids[0]; struct drm_encoder *encoder; int i;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] == 0) - break; - - encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); - if (!encoder) - continue; - + drm_connector_for_each_possible_encoder(connector, encoder, i) { if (encoder->encoder_type == DRM_MODE_ENCODER_VIRTUAL) return encoder; }
/* pick the first one */ - if (enc_id) - return drm_encoder_find(connector->dev, NULL, enc_id); + drm_connector_for_each_possible_encoder(connector, encoder, i) + return encoder; + return NULL; }
From: Ville Syrjälä ville.syrjala@linux.intel.com
Use drm_connector_for_each_possible_encoder() for iterating connector->encoder_ids[]. A bit more convenient not having to deal with the implementation details.
v2: Replace drm_for_each_connector_encoder_ids() with drm_connector_for_each_possible_encoder() (Daniel)
Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Ben Skeggs bskeggs@redhat.com Cc: nouveau@lists.freedesktop.org Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Alex Deucher alexander.deucher@amd.com #v1 --- drivers/gpu/drm/nouveau/nouveau_connector.c | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c index 7b557c354307..28d7b42cd666 100644 --- a/drivers/gpu/drm/nouveau/nouveau_connector.c +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c @@ -363,19 +363,11 @@ module_param_named(hdmimhz, nouveau_hdmimhz, int, 0400); struct nouveau_encoder * find_encoder(struct drm_connector *connector, int type) { - struct drm_device *dev = connector->dev; struct nouveau_encoder *nv_encoder; struct drm_encoder *enc; - int i, id; - - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - id = connector->encoder_ids[i]; - if (!id) - break; + int i;
- enc = drm_encoder_find(dev, NULL, id); - if (!enc) - continue; + drm_connector_for_each_possible_encoder(connector, enc, i) { nv_encoder = nouveau_encoder(enc);
if (type == DCB_OUTPUT_ANY || @@ -436,14 +428,7 @@ nouveau_connector_ddc_detect(struct drm_connector *connector) } }
- for (i = 0; nv_encoder = NULL, i < DRM_CONNECTOR_MAX_ENCODER; i++) { - int id = connector->encoder_ids[i]; - if (id == 0) - break; - - encoder = drm_encoder_find(dev, NULL, id); - if (!encoder) - continue; + drm_connector_for_each_possible_encoder(connector, encoder, i) { nv_encoder = nouveau_encoder(encoder);
if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
Hi Ville,
Thank you for the patch! Perhaps something to improve:
url: https://github.com/0day-ci/linux/commits/Ville-Syrjala/drm-Third-attempt-at-... base: git://people.freedesktop.org/~airlied/linux.git drm-next
smatch warnings: drivers/gpu/drm/nouveau/nouveau_connector.c:461 nouveau_connector_ddc_detect() error: uninitialized symbol 'nv_encoder'.
# https://github.com/0day-ci/linux/commit/7ec8bb65386edfb0b2bdc8e8391eb5e6eac4... git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout 7ec8bb65386edfb0b2bdc8e8391eb5e6eac44c06 vim +/nv_encoder +461 drivers/gpu/drm/nouveau/nouveau_connector.c
6ee738610 Ben Skeggs 2009-12-11 407 8777c5c11 Ben Skeggs 2014-06-06 408 static struct nouveau_encoder * 8777c5c11 Ben Skeggs 2014-06-06 409 nouveau_connector_ddc_detect(struct drm_connector *connector) 6ee738610 Ben Skeggs 2009-12-11 410 { 6ee738610 Ben Skeggs 2009-12-11 411 struct drm_device *dev = connector->dev; 1a1841d30 Ben Skeggs 2012-12-10 412 struct nouveau_connector *nv_connector = nouveau_connector(connector); 77145f1cb Ben Skeggs 2012-07-31 413 struct nouveau_drm *drm = nouveau_drm(dev); 1167c6bc5 Ben Skeggs 2016-05-18 414 struct nvkm_gpio *gpio = nvxx_gpio(&drm->client.device); 8777c5c11 Ben Skeggs 2014-06-06 415 struct nouveau_encoder *nv_encoder; 6d385c0aa Rob Clark 2014-07-17 416 struct drm_encoder *encoder; 1a1841d30 Ben Skeggs 2012-12-10 417 int i, panel = -ENODEV; 1a1841d30 Ben Skeggs 2012-12-10 418 1a1841d30 Ben Skeggs 2012-12-10 419 /* eDP panels need powering on by us (if the VBIOS doesn't default it 1a1841d30 Ben Skeggs 2012-12-10 420 * to on) before doing any AUX channel transactions. LVDS panel power 1a1841d30 Ben Skeggs 2012-12-10 421 * is handled by the SOR itself, and not required for LVDS DDC. 1a1841d30 Ben Skeggs 2012-12-10 422 */ 1a1841d30 Ben Skeggs 2012-12-10 423 if (nv_connector->type == DCB_CONNECTOR_eDP) { 2ea7249fe Ben Skeggs 2015-08-20 424 panel = nvkm_gpio_get(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff); 1a1841d30 Ben Skeggs 2012-12-10 425 if (panel == 0) { 2ea7249fe Ben Skeggs 2015-08-20 426 nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, 1); 1a1841d30 Ben Skeggs 2012-12-10 427 msleep(300); 1a1841d30 Ben Skeggs 2012-12-10 428 } 1a1841d30 Ben Skeggs 2012-12-10 429 } 6ee738610 Ben Skeggs 2009-12-11 430 7ec8bb653 Ville Syrjälä 2018-06-28 431 drm_connector_for_each_possible_encoder(connector, encoder, i) { 6d385c0aa Rob Clark 2014-07-17 432 nv_encoder = nouveau_encoder(encoder); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ If we enter the loop that means nv_encoder is non-NULL. Smatch can't prove that we always enter the loop in this case for whatever reason but my guess is that we always do.
4ca2b7120 Francisco Jerez 2010-08-08 433 8777c5c11 Ben Skeggs 2014-06-06 434 if (nv_encoder->dcb->type == DCB_OUTPUT_DP) { 8777c5c11 Ben Skeggs 2014-06-06 435 int ret = nouveau_dp_detect(nv_encoder); 52aa30f25 Ben Skeggs 2016-11-04 436 if (ret == NOUVEAU_DP_MST) 52aa30f25 Ben Skeggs 2016-11-04 437 return NULL; 52aa30f25 Ben Skeggs 2016-11-04 438 if (ret == NOUVEAU_DP_SST) 8777c5c11 Ben Skeggs 2014-06-06 439 break; 8777c5c11 Ben Skeggs 2014-06-06 440 } else 39c1c9011 Lukas Wunner 2016-01-11 441 if ((vga_switcheroo_handler_flags() & 39c1c9011 Lukas Wunner 2016-01-11 442 VGA_SWITCHEROO_CAN_SWITCH_DDC) && 39c1c9011 Lukas Wunner 2016-01-11 443 nv_encoder->dcb->type == DCB_OUTPUT_LVDS && 39c1c9011 Lukas Wunner 2016-01-11 444 nv_encoder->i2c) { 39c1c9011 Lukas Wunner 2016-01-11 445 int ret; 39c1c9011 Lukas Wunner 2016-01-11 446 vga_switcheroo_lock_ddc(dev->pdev); 39c1c9011 Lukas Wunner 2016-01-11 447 ret = nvkm_probe_i2c(nv_encoder->i2c, 0x50); 39c1c9011 Lukas Wunner 2016-01-11 448 vga_switcheroo_unlock_ddc(dev->pdev); 39c1c9011 Lukas Wunner 2016-01-11 449 if (ret) 39c1c9011 Lukas Wunner 2016-01-11 450 break; 39c1c9011 Lukas Wunner 2016-01-11 451 } else 8777c5c11 Ben Skeggs 2014-06-06 452 if (nv_encoder->i2c) { 2aa5eac51 Ben Skeggs 2015-08-20 453 if (nvkm_probe_i2c(nv_encoder->i2c, 0x50)) 1a1841d30 Ben Skeggs 2012-12-10 454 break; 6ee738610 Ben Skeggs 2009-12-11 455 } 6ee738610 Ben Skeggs 2009-12-11 456 } 6ee738610 Ben Skeggs 2009-12-11 457 1a1841d30 Ben Skeggs 2012-12-10 458 /* eDP panel not detected, restore panel power GPIO to previous 1a1841d30 Ben Skeggs 2012-12-10 459 * state to avoid confusing the SOR for other output types. 1a1841d30 Ben Skeggs 2012-12-10 460 */ 8777c5c11 Ben Skeggs 2014-06-06 @461 if (!nv_encoder && panel == 0) ^^^^^^^^^^^ So testing for NULL doesn't make sense. It's either non-NULL if we entered the loop at least once or it's uninitialized if we didn't enter the loop.
2ea7249fe Ben Skeggs 2015-08-20 462 nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, panel); 1a1841d30 Ben Skeggs 2012-12-10 463 8777c5c11 Ben Skeggs 2014-06-06 464 return nv_encoder; 6ee738610 Ben Skeggs 2009-12-11 465 } 6ee738610 Ben Skeggs 2009-12-11 466
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Sat, Jun 30, 2018 at 10:12:21PM +0300, Dan Carpenter wrote:
Hi Ville,
Thank you for the patch! Perhaps something to improve:
url: https://github.com/0day-ci/linux/commits/Ville-Syrjala/drm-Third-attempt-at-... base: git://people.freedesktop.org/~airlied/linux.git drm-next
smatch warnings: drivers/gpu/drm/nouveau/nouveau_connector.c:461 nouveau_connector_ddc_detect() error: uninitialized symbol 'nv_encoder'.
# https://github.com/0day-ci/linux/commit/7ec8bb65386edfb0b2bdc8e8391eb5e6eac4... git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout 7ec8bb65386edfb0b2bdc8e8391eb5e6eac44c06 vim +/nv_encoder +461 drivers/gpu/drm/nouveau/nouveau_connector.c
6ee738610 Ben Skeggs 2009-12-11 407 8777c5c11 Ben Skeggs 2014-06-06 408 static struct nouveau_encoder * 8777c5c11 Ben Skeggs 2014-06-06 409 nouveau_connector_ddc_detect(struct drm_connector *connector) 6ee738610 Ben Skeggs 2009-12-11 410 { 6ee738610 Ben Skeggs 2009-12-11 411 struct drm_device *dev = connector->dev; 1a1841d30 Ben Skeggs 2012-12-10 412 struct nouveau_connector *nv_connector = nouveau_connector(connector); 77145f1cb Ben Skeggs 2012-07-31 413 struct nouveau_drm *drm = nouveau_drm(dev); 1167c6bc5 Ben Skeggs 2016-05-18 414 struct nvkm_gpio *gpio = nvxx_gpio(&drm->client.device); 8777c5c11 Ben Skeggs 2014-06-06 415 struct nouveau_encoder *nv_encoder; 6d385c0aa Rob Clark 2014-07-17 416 struct drm_encoder *encoder; 1a1841d30 Ben Skeggs 2012-12-10 417 int i, panel = -ENODEV; 1a1841d30 Ben Skeggs 2012-12-10 418 1a1841d30 Ben Skeggs 2012-12-10 419 /* eDP panels need powering on by us (if the VBIOS doesn't default it 1a1841d30 Ben Skeggs 2012-12-10 420 * to on) before doing any AUX channel transactions. LVDS panel power 1a1841d30 Ben Skeggs 2012-12-10 421 * is handled by the SOR itself, and not required for LVDS DDC. 1a1841d30 Ben Skeggs 2012-12-10 422 */ 1a1841d30 Ben Skeggs 2012-12-10 423 if (nv_connector->type == DCB_CONNECTOR_eDP) { 2ea7249fe Ben Skeggs 2015-08-20 424 panel = nvkm_gpio_get(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff); 1a1841d30 Ben Skeggs 2012-12-10 425 if (panel == 0) { 2ea7249fe Ben Skeggs 2015-08-20 426 nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, 1); 1a1841d30 Ben Skeggs 2012-12-10 427 msleep(300); 1a1841d30 Ben Skeggs 2012-12-10 428 } 1a1841d30 Ben Skeggs 2012-12-10 429 } 6ee738610 Ben Skeggs 2009-12-11 430 7ec8bb653 Ville Syrjälä 2018-06-28 431 drm_connector_for_each_possible_encoder(connector, encoder, i) { 6d385c0aa Rob Clark 2014-07-17 432 nv_encoder = nouveau_encoder(encoder); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ If we enter the loop that means nv_encoder is non-NULL. Smatch can't prove that we always enter the loop in this case for whatever reason but my guess is that we always do.
4ca2b7120 Francisco Jerez 2010-08-08 433 8777c5c11 Ben Skeggs 2014-06-06 434 if (nv_encoder->dcb->type == DCB_OUTPUT_DP) { 8777c5c11 Ben Skeggs 2014-06-06 435 int ret = nouveau_dp_detect(nv_encoder); 52aa30f25 Ben Skeggs 2016-11-04 436 if (ret == NOUVEAU_DP_MST) 52aa30f25 Ben Skeggs 2016-11-04 437 return NULL; 52aa30f25 Ben Skeggs 2016-11-04 438 if (ret == NOUVEAU_DP_SST) 8777c5c11 Ben Skeggs 2014-06-06 439 break; 8777c5c11 Ben Skeggs 2014-06-06 440 } else 39c1c9011 Lukas Wunner 2016-01-11 441 if ((vga_switcheroo_handler_flags() & 39c1c9011 Lukas Wunner 2016-01-11 442 VGA_SWITCHEROO_CAN_SWITCH_DDC) && 39c1c9011 Lukas Wunner 2016-01-11 443 nv_encoder->dcb->type == DCB_OUTPUT_LVDS && 39c1c9011 Lukas Wunner 2016-01-11 444 nv_encoder->i2c) { 39c1c9011 Lukas Wunner 2016-01-11 445 int ret; 39c1c9011 Lukas Wunner 2016-01-11 446 vga_switcheroo_lock_ddc(dev->pdev); 39c1c9011 Lukas Wunner 2016-01-11 447 ret = nvkm_probe_i2c(nv_encoder->i2c, 0x50); 39c1c9011 Lukas Wunner 2016-01-11 448 vga_switcheroo_unlock_ddc(dev->pdev); 39c1c9011 Lukas Wunner 2016-01-11 449 if (ret) 39c1c9011 Lukas Wunner 2016-01-11 450 break; 39c1c9011 Lukas Wunner 2016-01-11 451 } else 8777c5c11 Ben Skeggs 2014-06-06 452 if (nv_encoder->i2c) { 2aa5eac51 Ben Skeggs 2015-08-20 453 if (nvkm_probe_i2c(nv_encoder->i2c, 0x50)) 1a1841d30 Ben Skeggs 2012-12-10 454 break; 6ee738610 Ben Skeggs 2009-12-11 455 } 6ee738610 Ben Skeggs 2009-12-11 456 } 6ee738610 Ben Skeggs 2009-12-11 457 1a1841d30 Ben Skeggs 2012-12-10 458 /* eDP panel not detected, restore panel power GPIO to previous 1a1841d30 Ben Skeggs 2012-12-10 459 * state to avoid confusing the SOR for other output types. 1a1841d30 Ben Skeggs 2012-12-10 460 */ 8777c5c11 Ben Skeggs 2014-06-06 @461 if (!nv_encoder && panel == 0) ^^^^^^^^^^^ So testing for NULL doesn't make sense. It's either non-NULL if we entered the loop at least once or it's uninitialized if we didn't enter the loop.
Yeah, seems like a bogus check. I suppose in theory you could have a connector with no encoders, although I can't think of any good reason why anyone would do that.
2ea7249fe Ben Skeggs 2015-08-20 462 nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, panel); 1a1841d30 Ben Skeggs 2012-12-10 463 8777c5c11 Ben Skeggs 2014-06-06 464 return nv_encoder; 6ee738610 Ben Skeggs 2009-12-11 465 } 6ee738610 Ben Skeggs 2009-12-11 466
0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Mon, Jul 02, 2018 at 04:04:45PM +0300, Ville Syrjälä wrote:
On Sat, Jun 30, 2018 at 10:12:21PM +0300, Dan Carpenter wrote:
Hi Ville,
Thank you for the patch! Perhaps something to improve:
url: https://github.com/0day-ci/linux/commits/Ville-Syrjala/drm-Third-attempt-at-... base: git://people.freedesktop.org/~airlied/linux.git drm-next
smatch warnings: drivers/gpu/drm/nouveau/nouveau_connector.c:461 nouveau_connector_ddc_detect() error: uninitialized symbol 'nv_encoder'.
# https://github.com/0day-ci/linux/commit/7ec8bb65386edfb0b2bdc8e8391eb5e6eac4... git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout 7ec8bb65386edfb0b2bdc8e8391eb5e6eac44c06 vim +/nv_encoder +461 drivers/gpu/drm/nouveau/nouveau_connector.c
6ee738610 Ben Skeggs 2009-12-11 407 8777c5c11 Ben Skeggs 2014-06-06 408 static struct nouveau_encoder * 8777c5c11 Ben Skeggs 2014-06-06 409 nouveau_connector_ddc_detect(struct drm_connector *connector) 6ee738610 Ben Skeggs 2009-12-11 410 { 6ee738610 Ben Skeggs 2009-12-11 411 struct drm_device *dev = connector->dev; 1a1841d30 Ben Skeggs 2012-12-10 412 struct nouveau_connector *nv_connector = nouveau_connector(connector); 77145f1cb Ben Skeggs 2012-07-31 413 struct nouveau_drm *drm = nouveau_drm(dev); 1167c6bc5 Ben Skeggs 2016-05-18 414 struct nvkm_gpio *gpio = nvxx_gpio(&drm->client.device); 8777c5c11 Ben Skeggs 2014-06-06 415 struct nouveau_encoder *nv_encoder; 6d385c0aa Rob Clark 2014-07-17 416 struct drm_encoder *encoder; 1a1841d30 Ben Skeggs 2012-12-10 417 int i, panel = -ENODEV; 1a1841d30 Ben Skeggs 2012-12-10 418 1a1841d30 Ben Skeggs 2012-12-10 419 /* eDP panels need powering on by us (if the VBIOS doesn't default it 1a1841d30 Ben Skeggs 2012-12-10 420 * to on) before doing any AUX channel transactions. LVDS panel power 1a1841d30 Ben Skeggs 2012-12-10 421 * is handled by the SOR itself, and not required for LVDS DDC. 1a1841d30 Ben Skeggs 2012-12-10 422 */ 1a1841d30 Ben Skeggs 2012-12-10 423 if (nv_connector->type == DCB_CONNECTOR_eDP) { 2ea7249fe Ben Skeggs 2015-08-20 424 panel = nvkm_gpio_get(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff); 1a1841d30 Ben Skeggs 2012-12-10 425 if (panel == 0) { 2ea7249fe Ben Skeggs 2015-08-20 426 nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, 1); 1a1841d30 Ben Skeggs 2012-12-10 427 msleep(300); 1a1841d30 Ben Skeggs 2012-12-10 428 } 1a1841d30 Ben Skeggs 2012-12-10 429 } 6ee738610 Ben Skeggs 2009-12-11 430 7ec8bb653 Ville Syrjälä 2018-06-28 431 drm_connector_for_each_possible_encoder(connector, encoder, i) { 6d385c0aa Rob Clark 2014-07-17 432 nv_encoder = nouveau_encoder(encoder); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ If we enter the loop that means nv_encoder is non-NULL. Smatch can't prove that we always enter the loop in this case for whatever reason but my guess is that we always do.
4ca2b7120 Francisco Jerez 2010-08-08 433 8777c5c11 Ben Skeggs 2014-06-06 434 if (nv_encoder->dcb->type == DCB_OUTPUT_DP) { 8777c5c11 Ben Skeggs 2014-06-06 435 int ret = nouveau_dp_detect(nv_encoder); 52aa30f25 Ben Skeggs 2016-11-04 436 if (ret == NOUVEAU_DP_MST) 52aa30f25 Ben Skeggs 2016-11-04 437 return NULL; 52aa30f25 Ben Skeggs 2016-11-04 438 if (ret == NOUVEAU_DP_SST) 8777c5c11 Ben Skeggs 2014-06-06 439 break; 8777c5c11 Ben Skeggs 2014-06-06 440 } else 39c1c9011 Lukas Wunner 2016-01-11 441 if ((vga_switcheroo_handler_flags() & 39c1c9011 Lukas Wunner 2016-01-11 442 VGA_SWITCHEROO_CAN_SWITCH_DDC) && 39c1c9011 Lukas Wunner 2016-01-11 443 nv_encoder->dcb->type == DCB_OUTPUT_LVDS && 39c1c9011 Lukas Wunner 2016-01-11 444 nv_encoder->i2c) { 39c1c9011 Lukas Wunner 2016-01-11 445 int ret; 39c1c9011 Lukas Wunner 2016-01-11 446 vga_switcheroo_lock_ddc(dev->pdev); 39c1c9011 Lukas Wunner 2016-01-11 447 ret = nvkm_probe_i2c(nv_encoder->i2c, 0x50); 39c1c9011 Lukas Wunner 2016-01-11 448 vga_switcheroo_unlock_ddc(dev->pdev); 39c1c9011 Lukas Wunner 2016-01-11 449 if (ret) 39c1c9011 Lukas Wunner 2016-01-11 450 break; 39c1c9011 Lukas Wunner 2016-01-11 451 } else 8777c5c11 Ben Skeggs 2014-06-06 452 if (nv_encoder->i2c) { 2aa5eac51 Ben Skeggs 2015-08-20 453 if (nvkm_probe_i2c(nv_encoder->i2c, 0x50)) 1a1841d30 Ben Skeggs 2012-12-10 454 break; 6ee738610 Ben Skeggs 2009-12-11 455 } 6ee738610 Ben Skeggs 2009-12-11 456 } 6ee738610 Ben Skeggs 2009-12-11 457 1a1841d30 Ben Skeggs 2012-12-10 458 /* eDP panel not detected, restore panel power GPIO to previous 1a1841d30 Ben Skeggs 2012-12-10 459 * state to avoid confusing the SOR for other output types. 1a1841d30 Ben Skeggs 2012-12-10 460 */ 8777c5c11 Ben Skeggs 2014-06-06 @461 if (!nv_encoder && panel == 0) ^^^^^^^^^^^ So testing for NULL doesn't make sense. It's either non-NULL if we entered the loop at least once or it's uninitialized if we didn't enter the loop.
Yeah, seems like a bogus check. I suppose in theory you could have a connector with no encoders, although I can't think of any good reason why anyone would do that.
In fact this thing seems to upset gcc as well. It's not particularly helpful in its warning message though:
../drivers/gpu/drm/nouveau/nouveau_connector.c: In function ‘nouveau_connector_detect’: ../drivers/gpu/drm/nouveau/nouveau_connector.c:597:33: warning: ‘nv_encoder’ may be used uninitialized in this function [-Wmaybe-uninitialized] if (nv_partner && ((nv_encoder->dcb->type == DCB_OUTPUT_ANALOG && ~~~~~~~~~~^~~~~
No indication that it's nouveau_connector_ddc_detect() that supposedly supplies the uninitialized pointer. I think I'll just toss in a =NULL in there to silence this noise.
2ea7249fe Ben Skeggs 2015-08-20 462 nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, panel); 1a1841d30 Ben Skeggs 2012-12-10 463 8777c5c11 Ben Skeggs 2014-06-06 464 return nv_encoder; 6ee738610 Ben Skeggs 2009-12-11 465 } 6ee738610 Ben Skeggs 2009-12-11 466
0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
-- Ville Syrjälä Intel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
From: Ville Syrjälä ville.syrjala@linux.intel.com
Use drm_connector_for_each_possible_encoder() for iterating connector->encoder_ids[]. A bit more convenient not having to deal with the implementation details.
v2: Replace drm_for_each_connector_encoder_ids() with drm_connector_for_each_possible_encoder() (Daniel) v3: Initialize nv_encoder to NULL to shut up gcc/smatch
Cc: Dan Carpenter dan.carpenter@oracle.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Ben Skeggs bskeggs@redhat.com Cc: nouveau@lists.freedesktop.org Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Alex Deucher alexander.deucher@amd.com #v1 --- drivers/gpu/drm/nouveau/nouveau_connector.c | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c index 7b557c354307..bb46c1d489cf 100644 --- a/drivers/gpu/drm/nouveau/nouveau_connector.c +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c @@ -363,19 +363,11 @@ module_param_named(hdmimhz, nouveau_hdmimhz, int, 0400); struct nouveau_encoder * find_encoder(struct drm_connector *connector, int type) { - struct drm_device *dev = connector->dev; struct nouveau_encoder *nv_encoder; struct drm_encoder *enc; - int i, id; - - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - id = connector->encoder_ids[i]; - if (!id) - break; + int i;
- enc = drm_encoder_find(dev, NULL, id); - if (!enc) - continue; + drm_connector_for_each_possible_encoder(connector, enc, i) { nv_encoder = nouveau_encoder(enc);
if (type == DCB_OUTPUT_ANY || @@ -420,7 +412,7 @@ nouveau_connector_ddc_detect(struct drm_connector *connector) struct nouveau_connector *nv_connector = nouveau_connector(connector); struct nouveau_drm *drm = nouveau_drm(dev); struct nvkm_gpio *gpio = nvxx_gpio(&drm->client.device); - struct nouveau_encoder *nv_encoder; + struct nouveau_encoder *nv_encoder = NULL; struct drm_encoder *encoder; int i, panel = -ENODEV;
@@ -436,14 +428,7 @@ nouveau_connector_ddc_detect(struct drm_connector *connector) } }
- for (i = 0; nv_encoder = NULL, i < DRM_CONNECTOR_MAX_ENCODER; i++) { - int id = connector->encoder_ids[i]; - if (id == 0) - break; - - encoder = drm_encoder_find(dev, NULL, id); - if (!encoder) - continue; + drm_connector_for_each_possible_encoder(connector, encoder, i) { nv_encoder = nouveau_encoder(encoder);
if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
From: Ville Syrjälä ville.syrjala@linux.intel.com
Use drm_connector_for_each_possible_encoder() for iterating connector->encoder_ids[]. A bit more convenient not having to deal with the implementation details.
v2: Replace drm_for_each_connector_encoder_ids() with drm_connector_for_each_possible_encoder() (Daniel)
Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Alex Deucher alexander.deucher@amd.com Cc: "Christian König" christian.koenig@amd.com Cc: "David (ChunMing) Zhou" David1.Zhou@amd.com Cc: Harry Wentland harry.wentland@amd.com Cc: amd-gfx@lists.freedesktop.org Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Alex Deucher alexander.deucher@amd.com #v1 --- drivers/gpu/drm/radeon/radeon_connectors.c | 90 +++++++++--------------------- 1 file changed, 26 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index 2aea2bdff99b..0655698f2956 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -244,23 +244,15 @@ radeon_connector_update_scratch_regs(struct drm_connector *connector, enum drm_c { struct drm_device *dev = connector->dev; struct radeon_device *rdev = dev->dev_private; - struct drm_encoder *best_encoder = NULL; - struct drm_encoder *encoder = NULL; + struct drm_encoder *best_encoder; + 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);
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] == 0) - break; - - encoder = drm_encoder_find(connector->dev, NULL, - connector->encoder_ids[i]); - if (!encoder) - continue; - + drm_connector_for_each_possible_encoder(connector, encoder, i) { if ((encoder == best_encoder) && (status == connector_status_connected)) connected = true; else @@ -270,7 +262,6 @@ radeon_connector_update_scratch_regs(struct drm_connector *connector, enum drm_c radeon_atombios_connected_scratch_regs(connector, encoder, connected); else radeon_combios_connected_scratch_regs(connector, encoder, connected); - } }
@@ -279,17 +270,11 @@ static struct drm_encoder *radeon_find_encoder(struct drm_connector *connector, struct drm_encoder *encoder; int i;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] == 0) - break; - - encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); - if (!encoder) - continue; - + drm_connector_for_each_possible_encoder(connector, encoder, i) { if (encoder->encoder_type == encoder_type) return encoder; } + return NULL; }
@@ -393,10 +378,13 @@ static int radeon_ddc_get_modes(struct drm_connector *connector)
static struct drm_encoder *radeon_best_single_encoder(struct drm_connector *connector) { - int enc_id = connector->encoder_ids[0]; - /* pick the encoder ids */ - if (enc_id) - return drm_encoder_find(connector->dev, NULL, enc_id); + struct drm_encoder *encoder; + int i; + + /* pick the first one */ + drm_connector_for_each_possible_encoder(connector, encoder, i) + return encoder; + return NULL; }
@@ -436,19 +424,19 @@ radeon_connector_analog_encoder_conflict_solve(struct drm_connector *connector, struct drm_device *dev = connector->dev; struct drm_connector *conflict; struct radeon_connector *radeon_conflict; - int i;
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); - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (conflict->encoder_ids[i] == 0) - break;
+ drm_connector_for_each_possible_encoder(conflict, enc, i) { /* if the IDs match */ - if (conflict->encoder_ids[i] == encoder->base.id) { + if (enc == encoder) { if (conflict->status != connector_status_connected) continue;
@@ -1256,7 +1244,7 @@ radeon_dvi_detect(struct drm_connector *connector, bool force) struct radeon_connector *radeon_connector = to_radeon_connector(connector); struct drm_encoder *encoder = NULL; const struct drm_encoder_helper_funcs *encoder_funcs; - int i, r; + int r; enum drm_connector_status ret = connector_status_disconnected; bool dret = false, broken_edid = false;
@@ -1374,15 +1362,9 @@ radeon_dvi_detect(struct drm_connector *connector, bool force)
/* find analog encoder */ if (radeon_connector->dac_load_detect) { - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] == 0) - break; - - encoder = drm_encoder_find(connector->dev, NULL, - connector->encoder_ids[i]); - if (!encoder) - continue; + int i;
+ drm_connector_for_each_possible_encoder(connector, encoder, i) { if (encoder->encoder_type != DRM_MODE_ENCODER_DAC && encoder->encoder_type != DRM_MODE_ENCODER_TVDAC) continue; @@ -1458,18 +1440,11 @@ radeon_dvi_detect(struct drm_connector *connector, bool force) /* okay need to be smart in here about which encoder to pick */ static struct drm_encoder *radeon_dvi_encoder(struct drm_connector *connector) { - int enc_id = connector->encoder_ids[0]; struct radeon_connector *radeon_connector = to_radeon_connector(connector); struct drm_encoder *encoder; int i; - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] == 0) - break; - - encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); - if (!encoder) - continue;
+ drm_connector_for_each_possible_encoder(connector, encoder, i) { if (radeon_connector->use_digital == true) { if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS) return encoder; @@ -1484,8 +1459,9 @@ static struct drm_encoder *radeon_dvi_encoder(struct drm_connector *connector)
/* then check use digitial */ /* pick the first one */ - if (enc_id) - return drm_encoder_find(connector->dev, NULL, enc_id); + drm_connector_for_each_possible_encoder(connector, encoder, i) + return encoder; + return NULL; }
@@ -1628,14 +1604,7 @@ u16 radeon_connector_encoder_get_dp_bridge_encoder_id(struct drm_connector *conn struct radeon_encoder *radeon_encoder; int i;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] == 0) - break; - - encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); - if (!encoder) - continue; - + drm_connector_for_each_possible_encoder(connector, encoder, i) { radeon_encoder = to_radeon_encoder(encoder);
switch (radeon_encoder->encoder_id) { @@ -1657,14 +1626,7 @@ static bool radeon_connector_encoder_is_hbr2(struct drm_connector *connector) int i; bool found = false;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] == 0) - break; - - encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); - if (!encoder) - continue; - + drm_connector_for_each_possible_encoder(connector, encoder, i) { radeon_encoder = to_radeon_encoder(encoder); if (radeon_encoder->caps & ATOM_ENCODER_CAP_RECORD_HBR2) found = true;
From: Ville Syrjälä ville.syrjala@linux.intel.com
Add a small helper for checking whether a connector and encoder are associated with each other.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_connector.c | 23 +++++++++++++++++++++++ include/drm/drm_connector.h | 3 +++ 2 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 186febcf4e55..84ff2fcdcb55 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -331,6 +331,29 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector, } EXPORT_SYMBOL(drm_mode_connector_attach_encoder);
+/** + * drm_connector_has_possible_encoder - check if the connector and encoder are assosicated with each other + * @connector: the connector + * @encoder: the encoder + * + * Returns: + * True if @encoder is one of the possible encoders for @connector. + */ +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; +} +EXPORT_SYMBOL(drm_connector_has_possible_encoder); + static void drm_mode_remove(struct drm_connector *connector, struct drm_display_mode *mode) { diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 8de3a3aa516a..9c59485fec36 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1181,6 +1181,9 @@ struct drm_connector * drm_connector_list_iter_next(struct drm_connector_list_iter *iter); void drm_connector_list_iter_end(struct drm_connector_list_iter *iter);
+bool drm_connector_has_possible_encoder(struct drm_connector *connector, + struct drm_encoder *encoder); + /** * drm_for_each_connector_iter - connector_list iterator macro * @connector: &struct drm_connector pointer used as cursor
From: Ville Syrjälä ville.syrjala@linux.intel.com
Use drm_connector_has_possible_encoder() for checking whether the encoder has an associated connector.
v2: Replace the drm_for_each_connector_encoder_ids() loop with a simple drm_connector_has_possible_encoder() call
Cc: Rob Clark robdclark@gmail.com Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/msm/dsi/dsi_manager.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c index 4cb1cb68878b..4beba3f7d067 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c @@ -751,12 +751,8 @@ struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id) connector_list = &dev->mode_config.connector_list;
list_for_each_entry(connector, connector_list, head) { - int i; - - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] == encoder->base.id) - return connector; - } + if (drm_connector_has_possible_encoder(connector, encoder)) + return connector; }
return ERR_PTR(-ENODEV);
From: Ville Syrjälä ville.syrjala@linux.intel.com
Use drm_connector_has_possible_encoder() for checking whether the encoder has an associated connector.
v2: Replace the drm_for_each_connector_encoder_ids() loop with a simple drm_connector_has_possible_encoder() call
Cc: Jyri Sarha jsarha@ti.com Cc: Tomi Valkeinen tomi.valkeinen@ti.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/tilcdc/tilcdc_external.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c index d651bdd6597e..b4eaf9bc87f8 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c @@ -103,12 +103,11 @@ struct drm_connector *tilcdc_encoder_find_connector(struct drm_device *ddev, struct drm_encoder *encoder) { struct drm_connector *connector; - int i;
- list_for_each_entry(connector, &ddev->mode_config.connector_list, head) - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) - if (connector->encoder_ids[i] == encoder->base.id) - return connector; + list_for_each_entry(connector, &ddev->mode_config.connector_list, head) { + if (drm_connector_has_possible_encoder(connector, encoder)) + return connector; + }
dev_err(ddev->dev, "No connector found for %s encoder (id %d)\n", encoder->name, encoder->base.id);
On 28/06/18 16:13, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Use drm_connector_has_possible_encoder() for checking whether the encoder has an associated connector.
v2: Replace the drm_for_each_connector_encoder_ids() loop with a simple drm_connector_has_possible_encoder() call
Cc: Jyri Sarha jsarha@ti.com Cc: Tomi Valkeinen tomi.valkeinen@ti.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Acked-by: Jyri Sarha jsarha@ti.com
I guess you are going to merge these at one go trough drm-misc.
drivers/gpu/drm/tilcdc/tilcdc_external.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c index d651bdd6597e..b4eaf9bc87f8 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c @@ -103,12 +103,11 @@ struct drm_connector *tilcdc_encoder_find_connector(struct drm_device *ddev, struct drm_encoder *encoder) { struct drm_connector *connector;
int i;
list_for_each_entry(connector, &ddev->mode_config.connector_list, head)
for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)if (connector->encoder_ids[i] == encoder->base.id)return connector;
list_for_each_entry(connector, &ddev->mode_config.connector_list, head) {
if (drm_connector_has_possible_encoder(connector, encoder))return connector;}
dev_err(ddev->dev, "No connector found for %s encoder (id %d)\n", encoder->name, encoder->base.id);
On Thu, Jun 28, 2018 at 04:45:50PM +0300, Jyri Sarha wrote:
On 28/06/18 16:13, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Use drm_connector_has_possible_encoder() for checking whether the encoder has an associated connector.
v2: Replace the drm_for_each_connector_encoder_ids() loop with a simple drm_connector_has_possible_encoder() call
Cc: Jyri Sarha jsarha@ti.com Cc: Tomi Valkeinen tomi.valkeinen@ti.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Acked-by: Jyri Sarha jsarha@ti.com
I guess you are going to merge these at one go trough drm-misc.
That's the plan. Thanks for the ack.
drivers/gpu/drm/tilcdc/tilcdc_external.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c index d651bdd6597e..b4eaf9bc87f8 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c @@ -103,12 +103,11 @@ struct drm_connector *tilcdc_encoder_find_connector(struct drm_device *ddev, struct drm_encoder *encoder) { struct drm_connector *connector;
int i;
list_for_each_entry(connector, &ddev->mode_config.connector_list, head)
for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)if (connector->encoder_ids[i] == encoder->base.id)return connector;
list_for_each_entry(connector, &ddev->mode_config.connector_list, head) {
if (drm_connector_has_possible_encoder(connector, encoder))return connector;}
dev_err(ddev->dev, "No connector found for %s encoder (id %d)\n", encoder->name, encoder->base.id);
-- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On Thu, Jun 28, 2018 at 04:13:06PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Changes from the previous version mainly involve Danoie's suggestion
Can't type today either: "Daniel's"
of hiding the drm_encoder_find() in the iterator macro. I also polished the msm and tilcdc cases a bit more with another small helper.
Cc: Alex Deucher alexander.deucher@amd.com Cc: amd-gfx@lists.freedesktop.org Cc: Ben Skeggs bskeggs@redhat.com Cc: "Christian König" christian.koenig@amd.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: "David (ChunMing) Zhou" David1.Zhou@amd.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: freedreno@lists.freedesktop.org Cc: Harry Wentland harry.wentland@amd.com Cc: Jyri Sarha jsarha@ti.com Cc: linux-arm-msm@vger.kernel.org Cc: nouveau@lists.freedesktop.org Cc: Rob Clark robdclark@gmail.com Cc: Tomi Valkeinen tomi.valkeinen@ti.com
Ville Syrjälä (9): drm/fb-helper: Eliminate the .best_encoder() usage drm/i915: Nuke intel_mst_best_encoder() drm: Add drm_connector_for_each_possible_encoder() drm/amdgpu: Use drm_connector_for_each_possible_encoder() drm/nouveau: Use drm_connector_for_each_possible_encoder() drm/radeon: Use drm_connector_for_each_possible_encoder() drm: Add drm_connector_has_possible_encoder() drm/msm: Use drm_connector_has_possible_encoder() drm/tilcdc: Use drm_connector_has_possible_encoder()
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 81 ++++++----------------- drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 15 ++--- drivers/gpu/drm/drm_connector.c | 44 +++++++++---- drivers/gpu/drm/drm_fb_helper.c | 34 +++++----- drivers/gpu/drm/drm_probe_helper.c | 10 +-- drivers/gpu/drm/i915/intel_dp_mst.c | 10 --- drivers/gpu/drm/msm/dsi/dsi_manager.c | 8 +-- drivers/gpu/drm/nouveau/nouveau_connector.c | 21 +----- drivers/gpu/drm/radeon/radeon_connectors.c | 90 ++++++++------------------ drivers/gpu/drm/tilcdc/tilcdc_external.c | 9 ++- include/drm/drm_connector.h | 16 +++++ 11 files changed, 128 insertions(+), 210 deletions(-)
-- 2.16.4
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Dan Carpenter -
Daniel Vetter -
Jyri Sarha -
Ville Syrjala -
Ville Syrjälä