* [PATCH 1/8] drm/fb-helper: Eliminate the .best_encoder() usage
2018-06-26 17:47 [PATCH 0/8] drm: Second attempt at fixing the fb-helper .best_encoder() mess Ville Syrjala
@ 2018-06-26 17:47 ` Ville Syrjala
2018-06-26 17:58 ` Alex Deucher
2018-06-27 9:03 ` Daniel Vetter
2018-06-26 17:47 ` [PATCH 2/8] drm/i915: Nuke intel_mst_best_encoder() Ville Syrjala
` (7 subsequent siblings)
8 siblings, 2 replies; 21+ messages in thread
From: Ville Syrjala @ 2018-06-26 17:47 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, intel-gfx, Harry Wentland, Dhinakaran Pandiyan
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.
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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 | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index cab14f253384..61c39cd75a27 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2323,6 +2323,23 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
return true;
}
+static bool connector_crtc_ok(struct drm_connector *connector,
+ struct drm_crtc *crtc)
+{
+ int i;
+
+ for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
+ struct drm_encoder *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 +2348,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 +2378,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 +2385,7 @@ 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_crtc_ok(connector, crtc->mode_set.crtc))
continue;
for (o = 0; o < n; o++)
@@ -2410,7 +2412,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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 1/8] drm/fb-helper: Eliminate the .best_encoder() usage
2018-06-26 17:47 ` [PATCH 1/8] drm/fb-helper: Eliminate the .best_encoder() usage Ville Syrjala
@ 2018-06-26 17:58 ` Alex Deucher
2018-06-27 9:03 ` Daniel Vetter
1 sibling, 0 replies; 21+ messages in thread
From: Alex Deucher @ 2018-06-26 17:58 UTC (permalink / raw)
To: Ville Syrjala
Cc: Daniel Vetter, Intel Graphics Development, Dhinakaran Pandiyan,
Maling list - DRI developers
On Tue, Jun 26, 2018 at 1:47 PM, 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.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 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 | 36 +++++++++++++++++++-----------------
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index cab14f253384..61c39cd75a27 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2323,6 +2323,23 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
> return true;
> }
>
> +static bool connector_crtc_ok(struct drm_connector *connector,
> + struct drm_crtc *crtc)
> +{
> + int i;
> +
> + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> + struct drm_encoder *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 +2348,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 +2378,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 +2385,7 @@ 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_crtc_ok(connector, crtc->mode_set.crtc))
> continue;
>
> for (o = 0; o < n; o++)
> @@ -2410,7 +2412,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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/8] drm/fb-helper: Eliminate the .best_encoder() usage
2018-06-26 17:47 ` [PATCH 1/8] drm/fb-helper: Eliminate the .best_encoder() usage Ville Syrjala
2018-06-26 17:58 ` Alex Deucher
@ 2018-06-27 9:03 ` Daniel Vetter
2018-06-27 12:03 ` Ville Syrjälä
1 sibling, 1 reply; 21+ messages in thread
From: Daniel Vetter @ 2018-06-27 9:03 UTC (permalink / raw)
To: Ville Syrjala; +Cc: Daniel Vetter, intel-gfx, Dhinakaran Pandiyan, dri-devel
On Tue, Jun 26, 2018 at 08:47:07PM +0300, Ville Syrjala 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.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 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 | 36 +++++++++++++++++++-----------------
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index cab14f253384..61c39cd75a27 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2323,6 +2323,23 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
> return true;
> }
>
> +static bool connector_crtc_ok(struct drm_connector *connector,
> + struct drm_crtc *crtc)
> +{
> + int i;
> +
> + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> + struct drm_encoder *encoder =
> + drm_encoder_find(connector->dev, NULL,
> + connector->encoder_ids[i]);
> +
Shouldn't we also check for encoder != NULL here? Just for the case where
a connector does not work on the given crtc, where we do expect to
actually run off the end of the valid entries in the array.
With that fixed:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> + 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 +2348,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 +2378,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 +2385,7 @@ 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_crtc_ok(connector, crtc->mode_set.crtc))
> continue;
>
> for (o = 0; o < n; o++)
> @@ -2410,7 +2412,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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/8] drm/fb-helper: Eliminate the .best_encoder() usage
2018-06-27 9:03 ` Daniel Vetter
@ 2018-06-27 12:03 ` Ville Syrjälä
0 siblings, 0 replies; 21+ messages in thread
From: Ville Syrjälä @ 2018-06-27 12:03 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, intel-gfx, Dhinakaran Pandiyan, dri-devel
On Wed, Jun 27, 2018 at 11:03:31AM +0200, Daniel Vetter wrote:
> On Tue, Jun 26, 2018 at 08:47:07PM +0300, Ville Syrjala 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.
> >
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Harry Wentland <harry.wentland@amd.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 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 | 36 +++++++++++++++++++-----------------
> > 1 file changed, 19 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index cab14f253384..61c39cd75a27 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -2323,6 +2323,23 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
> > return true;
> > }
> >
> > +static bool connector_crtc_ok(struct drm_connector *connector,
> > + struct drm_crtc *crtc)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> > + struct drm_encoder *encoder =
> > + drm_encoder_find(connector->dev, NULL,
> > + connector->encoder_ids[i]);
> > +
>
> Shouldn't we also check for encoder != NULL here? Just for the case where
> a connector does not work on the given crtc, where we do expect to
> actually run off the end of the valid entries in the array.
Yeah. This one is obviously crap. Originally I had it after the
for_each_encoder_ids() thing so didn't need any checks, but then
thought I should move it before just in case people don't like
that particular macro. And apparently forgot to think when doing
that.
>
> With that fixed:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> > + 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 +2348,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 +2378,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 +2385,7 @@ 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_crtc_ok(connector, crtc->mode_set.crtc))
> > continue;
> >
> > for (o = 0; o < n; o++)
> > @@ -2410,7 +2412,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
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/8] drm/i915: Nuke intel_mst_best_encoder()
2018-06-26 17:47 [PATCH 0/8] drm: Second attempt at fixing the fb-helper .best_encoder() mess Ville Syrjala
2018-06-26 17:47 ` [PATCH 1/8] drm/fb-helper: Eliminate the .best_encoder() usage Ville Syrjala
@ 2018-06-26 17:47 ` Ville Syrjala
2018-06-27 9:03 ` Daniel Vetter
2018-06-26 17:47 ` [PATCH 3/8] drm: Add drm_for_each_connector_encoder_ids() Ville Syrjala
` (6 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjala @ 2018-06-26 17:47 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Dhinakaran Pandiyan
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: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.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,
};
--
2.16.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 2/8] drm/i915: Nuke intel_mst_best_encoder()
2018-06-26 17:47 ` [PATCH 2/8] drm/i915: Nuke intel_mst_best_encoder() Ville Syrjala
@ 2018-06-27 9:03 ` Daniel Vetter
0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2018-06-27 9:03 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx, Dhinakaran Pandiyan, dri-devel
On Tue, Jun 26, 2018 at 08:47:08PM +0300, Ville Syrjala wrote:
> 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: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Yay!
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> 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,
> };
>
> --
> 2.16.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/8] drm: Add drm_for_each_connector_encoder_ids()
2018-06-26 17:47 [PATCH 0/8] drm: Second attempt at fixing the fb-helper .best_encoder() mess Ville Syrjala
2018-06-26 17:47 ` [PATCH 1/8] drm/fb-helper: Eliminate the .best_encoder() usage Ville Syrjala
2018-06-26 17:47 ` [PATCH 2/8] drm/i915: Nuke intel_mst_best_encoder() Ville Syrjala
@ 2018-06-26 17:47 ` Ville Syrjala
2018-06-27 9:08 ` Daniel Vetter
2018-06-26 17:47 ` [PATCH 6/8] drm/nouveau: Use drm_for_each_connector_encoder_ids() Ville Syrjala
` (5 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjala @ 2018-06-26 17:47 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
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.
Also use ARRAY_SIZE() when populating the array to avoid spreading
knowledge about the array size all over.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_connector.c | 22 ++++++++++------------
drivers/gpu/drm/drm_fb_helper.c | 6 +++---
drivers/gpu/drm/drm_probe_helper.c | 9 +++++----
include/drm/drm_connector.h | 11 +++++++++++
4 files changed, 29 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2f9ebddd178e..c43646cb8145 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;
@@ -1693,6 +1693,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
int encoders_count = 0;
int ret = 0;
int copied = 0;
+ u32 encoder_id;
int i;
struct drm_mode_modeinfo u_mode;
struct drm_mode_modeinfo __user *mode_ptr;
@@ -1708,22 +1709,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_for_each_connector_encoder_ids(connector, encoder_id, 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_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ if (put_user(encoder_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 61c39cd75a27..e086b08748f4 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2326,12 +2326,12 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
static bool connector_crtc_ok(struct drm_connector *connector,
struct drm_crtc *crtc)
{
+ u32 encoder_id;
int i;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
struct drm_encoder *encoder =
- drm_encoder_find(connector->dev, NULL,
- connector->encoder_ids[i]);
+ drm_encoder_find(connector->dev, NULL, encoder_id);
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..0239f76c52fb 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;
+ u32 encoder_id;
+ int i;
/* Step 1: Validate against connector */
ret = drm_connector_mode_valid(connector, mode);
@@ -98,8 +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_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ struct drm_encoder *encoder =
+ drm_encoder_find(dev, NULL, encoder_id);
struct drm_crtc *crtc;
if (!encoder)
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 14ab58ade87f..4a0363f7dd8c 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1193,4 +1193,15 @@ 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_for_each_connector_encoder_ids - iterate connector encoder_ids[]
+ * @connector: &struct drm_connector pointer used as cursor
+ * @encoder_id: the encoder ID
+ * @__i: int iteration cursor, for macro-internal use
+ */
+#define drm_for_each_connector_encoder_ids(connector, encoder_id, __i) \
+ for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
+ (connector)->encoder_ids[(__i)] != 0; (__i)++) \
+ for_each_if((encoder_id) = (connector)->encoder_ids[(__i)])
+
#endif
--
2.16.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 3/8] drm: Add drm_for_each_connector_encoder_ids()
2018-06-26 17:47 ` [PATCH 3/8] drm: Add drm_for_each_connector_encoder_ids() Ville Syrjala
@ 2018-06-27 9:08 ` Daniel Vetter
2018-06-27 9:11 ` Daniel Vetter
0 siblings, 1 reply; 21+ messages in thread
From: Daniel Vetter @ 2018-06-27 9:08 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx, dri-devel
On Tue, Jun 26, 2018 at 08:47: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.
>
> Also use ARRAY_SIZE() when populating the array to avoid spreading
> knowledge about the array size all over.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/drm_connector.c | 22 ++++++++++------------
> drivers/gpu/drm/drm_fb_helper.c | 6 +++---
> drivers/gpu/drm/drm_probe_helper.c | 9 +++++----
> include/drm/drm_connector.h | 11 +++++++++++
> 4 files changed, 29 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2f9ebddd178e..c43646cb8145 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;
> @@ -1693,6 +1693,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> int encoders_count = 0;
> int ret = 0;
> int copied = 0;
> + u32 encoder_id;
> int i;
> struct drm_mode_modeinfo u_mode;
> struct drm_mode_modeinfo __user *mode_ptr;
> @@ -1708,22 +1709,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_for_each_connector_encoder_ids(connector, encoder_id, 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_for_each_connector_encoder_ids(connector, encoder_id, i) {
> + if (put_user(encoder_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 61c39cd75a27..e086b08748f4 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2326,12 +2326,12 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
> static bool connector_crtc_ok(struct drm_connector *connector,
> struct drm_crtc *crtc)
> {
> + u32 encoder_id;
> int i;
>
> - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> + drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
> struct drm_encoder *encoder =
> - drm_encoder_find(connector->dev, NULL,
> - connector->encoder_ids[i]);
> + drm_encoder_find(connector->dev, NULL, encoder_id);
>
> 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..0239f76c52fb 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;
> + u32 encoder_id;
> + int i;
>
> /* Step 1: Validate against connector */
> ret = drm_connector_mode_valid(connector, mode);
> @@ -98,8 +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_for_each_connector_encoder_ids(connector, encoder_id, i) {
> + struct drm_encoder *encoder =
> + drm_encoder_find(dev, NULL, encoder_id);
> struct drm_crtc *crtc;
>
> if (!encoder)
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 14ab58ade87f..4a0363f7dd8c 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1193,4 +1193,15 @@ 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_for_each_connector_encoder_ids - iterate connector encoder_ids[]
&drm_connector.encoder_ids would give you a (fairly direct) link. Just a
nitpick.
Wrt the api: I think including the drm_encoder_find would be useful here -
it's what 2/3 users want anyway, and the ioctl can easily look up the id
again by following encoder->base.id. And then maybe call the function a
bit differently, I think usually we go with
$parent_object_for_each_$iterated_thing, so
drm_connector_for_each_encoder(). There's also the
for_each_$thing_in_$parent_object pattern (used for atomic states), but I
think that's less fitting here.
Apologies for the interface bikeshedding ...
-Daniel
> + * @connector: &struct drm_connector pointer used as cursor
> + * @encoder_id: the encoder ID
> + * @__i: int iteration cursor, for macro-internal use
> + */
> +#define drm_for_each_connector_encoder_ids(connector, encoder_id, __i) \
> + for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
> + (connector)->encoder_ids[(__i)] != 0; (__i)++) \
> + for_each_if((encoder_id) = (connector)->encoder_ids[(__i)])
> +
> #endif
> --
> 2.16.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 3/8] drm: Add drm_for_each_connector_encoder_ids()
2018-06-27 9:08 ` Daniel Vetter
@ 2018-06-27 9:11 ` Daniel Vetter
2018-06-27 15:07 ` Ville Syrjälä
0 siblings, 1 reply; 21+ messages in thread
From: Daniel Vetter @ 2018-06-27 9:11 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx, dri-devel
On Wed, Jun 27, 2018 at 11:08:48AM +0200, Daniel Vetter wrote:
> On Tue, Jun 26, 2018 at 08:47: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.
> >
> > Also use ARRAY_SIZE() when populating the array to avoid spreading
> > knowledge about the array size all over.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/drm_connector.c | 22 ++++++++++------------
> > drivers/gpu/drm/drm_fb_helper.c | 6 +++---
> > drivers/gpu/drm/drm_probe_helper.c | 9 +++++----
> > include/drm/drm_connector.h | 11 +++++++++++
> > 4 files changed, 29 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index 2f9ebddd178e..c43646cb8145 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;
> > @@ -1693,6 +1693,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> > int encoders_count = 0;
> > int ret = 0;
> > int copied = 0;
> > + u32 encoder_id;
> > int i;
> > struct drm_mode_modeinfo u_mode;
> > struct drm_mode_modeinfo __user *mode_ptr;
> > @@ -1708,22 +1709,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_for_each_connector_encoder_ids(connector, encoder_id, 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_for_each_connector_encoder_ids(connector, encoder_id, i) {
> > + if (put_user(encoder_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 61c39cd75a27..e086b08748f4 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -2326,12 +2326,12 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
> > static bool connector_crtc_ok(struct drm_connector *connector,
> > struct drm_crtc *crtc)
> > {
> > + u32 encoder_id;
> > int i;
> >
> > - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> > + drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
> > struct drm_encoder *encoder =
> > - drm_encoder_find(connector->dev, NULL,
> > - connector->encoder_ids[i]);
> > + drm_encoder_find(connector->dev, NULL, encoder_id);
> >
> > 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..0239f76c52fb 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;
> > + u32 encoder_id;
> > + int i;
> >
> > /* Step 1: Validate against connector */
> > ret = drm_connector_mode_valid(connector, mode);
> > @@ -98,8 +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_for_each_connector_encoder_ids(connector, encoder_id, i) {
> > + struct drm_encoder *encoder =
> > + drm_encoder_find(dev, NULL, encoder_id);
> > struct drm_crtc *crtc;
> >
> > if (!encoder)
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 14ab58ade87f..4a0363f7dd8c 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -1193,4 +1193,15 @@ 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_for_each_connector_encoder_ids - iterate connector encoder_ids[]
>
> &drm_connector.encoder_ids would give you a (fairly direct) link. Just a
> nitpick.
>
> Wrt the api: I think including the drm_encoder_find would be useful here -
> it's what 2/3 users want anyway, and the ioctl can easily look up the id
> again by following encoder->base.id. And then maybe call the function a
> bit differently, I think usually we go with
> $parent_object_for_each_$iterated_thing, so
> drm_connector_for_each_encoder(). There's also the
> for_each_$thing_in_$parent_object pattern (used for atomic states), but I
> think that's less fitting here.
>
> Apologies for the interface bikeshedding ...
Looking at all the patches it's 15 places that want the drm_encoder_find
integrated vs. 4 that don't want it (without further refactoring). So even
more reasons for drm_connector_for_each_encoder imo.
-Daniel
> -Daniel
>
> > + * @connector: &struct drm_connector pointer used as cursor
> > + * @encoder_id: the encoder ID
> > + * @__i: int iteration cursor, for macro-internal use
> > + */
> > +#define drm_for_each_connector_encoder_ids(connector, encoder_id, __i) \
> > + for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
> > + (connector)->encoder_ids[(__i)] != 0; (__i)++) \
> > + for_each_if((encoder_id) = (connector)->encoder_ids[(__i)])
> > +
> > #endif
> > --
> > 2.16.4
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 3/8] drm: Add drm_for_each_connector_encoder_ids()
2018-06-27 9:11 ` Daniel Vetter
@ 2018-06-27 15:07 ` Ville Syrjälä
2018-06-28 6:04 ` Daniel Vetter
0 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjälä @ 2018-06-27 15:07 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
On Wed, Jun 27, 2018 at 11:11:57AM +0200, Daniel Vetter wrote:
> On Wed, Jun 27, 2018 at 11:08:48AM +0200, Daniel Vetter wrote:
> > On Tue, Jun 26, 2018 at 08:47: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.
> > >
> > > Also use ARRAY_SIZE() when populating the array to avoid spreading
> > > knowledge about the array size all over.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/drm_connector.c | 22 ++++++++++------------
> > > drivers/gpu/drm/drm_fb_helper.c | 6 +++---
> > > drivers/gpu/drm/drm_probe_helper.c | 9 +++++----
> > > include/drm/drm_connector.h | 11 +++++++++++
> > > 4 files changed, 29 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index 2f9ebddd178e..c43646cb8145 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;
> > > @@ -1693,6 +1693,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> > > int encoders_count = 0;
> > > int ret = 0;
> > > int copied = 0;
> > > + u32 encoder_id;
> > > int i;
> > > struct drm_mode_modeinfo u_mode;
> > > struct drm_mode_modeinfo __user *mode_ptr;
> > > @@ -1708,22 +1709,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_for_each_connector_encoder_ids(connector, encoder_id, 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_for_each_connector_encoder_ids(connector, encoder_id, i) {
> > > + if (put_user(encoder_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 61c39cd75a27..e086b08748f4 100644
> > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > @@ -2326,12 +2326,12 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
> > > static bool connector_crtc_ok(struct drm_connector *connector,
> > > struct drm_crtc *crtc)
> > > {
> > > + u32 encoder_id;
> > > int i;
> > >
> > > - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> > > + drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
> > > struct drm_encoder *encoder =
> > > - drm_encoder_find(connector->dev, NULL,
> > > - connector->encoder_ids[i]);
> > > + drm_encoder_find(connector->dev, NULL, encoder_id);
> > >
> > > 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..0239f76c52fb 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;
> > > + u32 encoder_id;
> > > + int i;
> > >
> > > /* Step 1: Validate against connector */
> > > ret = drm_connector_mode_valid(connector, mode);
> > > @@ -98,8 +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_for_each_connector_encoder_ids(connector, encoder_id, i) {
> > > + struct drm_encoder *encoder =
> > > + drm_encoder_find(dev, NULL, encoder_id);
> > > struct drm_crtc *crtc;
> > >
> > > if (!encoder)
> > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > index 14ab58ade87f..4a0363f7dd8c 100644
> > > --- a/include/drm/drm_connector.h
> > > +++ b/include/drm/drm_connector.h
> > > @@ -1193,4 +1193,15 @@ 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_for_each_connector_encoder_ids - iterate connector encoder_ids[]
> >
> > &drm_connector.encoder_ids would give you a (fairly direct) link. Just a
> > nitpick.
> >
> > Wrt the api: I think including the drm_encoder_find would be useful here -
> > it's what 2/3 users want anyway, and the ioctl can easily look up the id
> > again by following encoder->base.id. And then maybe call the function a
> > bit differently, I think usually we go with
> > $parent_object_for_each_$iterated_thing, so
> > drm_connector_for_each_encoder(). There's also the
> > for_each_$thing_in_$parent_object pattern (used for atomic states), but I
> > think that's less fitting here.
> >
> > Apologies for the interface bikeshedding ...
>
> Looking at all the patches it's 15 places that want the drm_encoder_find
> integrated vs. 4 that don't want it (without further refactoring). So even
> more reasons for drm_connector_for_each_encoder imo.
I had that same thought initially, but then forgot about it for
some reason. It does indeed lead to an even neater result.
Although it should be somewhat obvious that there can only be one
currently used encoder for a connector, I think we may want to name
this eg. drm_connector_for_each_possible_encoder() to make it super
clear what we're talking about.
> -Daniel
>
> > -Daniel
> >
> > > + * @connector: &struct drm_connector pointer used as cursor
> > > + * @encoder_id: the encoder ID
> > > + * @__i: int iteration cursor, for macro-internal use
> > > + */
> > > +#define drm_for_each_connector_encoder_ids(connector, encoder_id, __i) \
> > > + for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
> > > + (connector)->encoder_ids[(__i)] != 0; (__i)++) \
> > > + for_each_if((encoder_id) = (connector)->encoder_ids[(__i)])
> > > +
> > > #endif
> > > --
> > > 2.16.4
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 3/8] drm: Add drm_for_each_connector_encoder_ids()
2018-06-27 15:07 ` Ville Syrjälä
@ 2018-06-28 6:04 ` Daniel Vetter
0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2018-06-28 6:04 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, dri-devel
On Wed, Jun 27, 2018 at 06:07:39PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 27, 2018 at 11:11:57AM +0200, Daniel Vetter wrote:
> > On Wed, Jun 27, 2018 at 11:08:48AM +0200, Daniel Vetter wrote:
> > > On Tue, Jun 26, 2018 at 08:47: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.
> > > >
> > > > Also use ARRAY_SIZE() when populating the array to avoid spreading
> > > > knowledge about the array size all over.
> > > >
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > > drivers/gpu/drm/drm_connector.c | 22 ++++++++++------------
> > > > drivers/gpu/drm/drm_fb_helper.c | 6 +++---
> > > > drivers/gpu/drm/drm_probe_helper.c | 9 +++++----
> > > > include/drm/drm_connector.h | 11 +++++++++++
> > > > 4 files changed, 29 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > > index 2f9ebddd178e..c43646cb8145 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;
> > > > @@ -1693,6 +1693,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> > > > int encoders_count = 0;
> > > > int ret = 0;
> > > > int copied = 0;
> > > > + u32 encoder_id;
> > > > int i;
> > > > struct drm_mode_modeinfo u_mode;
> > > > struct drm_mode_modeinfo __user *mode_ptr;
> > > > @@ -1708,22 +1709,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_for_each_connector_encoder_ids(connector, encoder_id, 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_for_each_connector_encoder_ids(connector, encoder_id, i) {
> > > > + if (put_user(encoder_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 61c39cd75a27..e086b08748f4 100644
> > > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > > @@ -2326,12 +2326,12 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
> > > > static bool connector_crtc_ok(struct drm_connector *connector,
> > > > struct drm_crtc *crtc)
> > > > {
> > > > + u32 encoder_id;
> > > > int i;
> > > >
> > > > - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> > > > + drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
> > > > struct drm_encoder *encoder =
> > > > - drm_encoder_find(connector->dev, NULL,
> > > > - connector->encoder_ids[i]);
> > > > + drm_encoder_find(connector->dev, NULL, encoder_id);
> > > >
> > > > 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..0239f76c52fb 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;
> > > > + u32 encoder_id;
> > > > + int i;
> > > >
> > > > /* Step 1: Validate against connector */
> > > > ret = drm_connector_mode_valid(connector, mode);
> > > > @@ -98,8 +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_for_each_connector_encoder_ids(connector, encoder_id, i) {
> > > > + struct drm_encoder *encoder =
> > > > + drm_encoder_find(dev, NULL, encoder_id);
> > > > struct drm_crtc *crtc;
> > > >
> > > > if (!encoder)
> > > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > > index 14ab58ade87f..4a0363f7dd8c 100644
> > > > --- a/include/drm/drm_connector.h
> > > > +++ b/include/drm/drm_connector.h
> > > > @@ -1193,4 +1193,15 @@ 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_for_each_connector_encoder_ids - iterate connector encoder_ids[]
> > >
> > > &drm_connector.encoder_ids would give you a (fairly direct) link. Just a
> > > nitpick.
> > >
> > > Wrt the api: I think including the drm_encoder_find would be useful here -
> > > it's what 2/3 users want anyway, and the ioctl can easily look up the id
> > > again by following encoder->base.id. And then maybe call the function a
> > > bit differently, I think usually we go with
> > > $parent_object_for_each_$iterated_thing, so
> > > drm_connector_for_each_encoder(). There's also the
> > > for_each_$thing_in_$parent_object pattern (used for atomic states), but I
> > > think that's less fitting here.
> > >
> > > Apologies for the interface bikeshedding ...
> >
> > Looking at all the patches it's 15 places that want the drm_encoder_find
> > integrated vs. 4 that don't want it (without further refactoring). So even
> > more reasons for drm_connector_for_each_encoder imo.
>
> I had that same thought initially, but then forgot about it for
> some reason. It does indeed lead to an even neater result.
>
> Although it should be somewhat obvious that there can only be one
> currently used encoder for a connector, I think we may want to name
> this eg. drm_connector_for_each_possible_encoder() to make it super
> clear what we're talking about.
Yeah, that sounds like an even better name.
-Daniel
>
> > -Daniel
> >
> > > -Daniel
> > >
> > > > + * @connector: &struct drm_connector pointer used as cursor
> > > > + * @encoder_id: the encoder ID
> > > > + * @__i: int iteration cursor, for macro-internal use
> > > > + */
> > > > +#define drm_for_each_connector_encoder_ids(connector, encoder_id, __i) \
> > > > + for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
> > > > + (connector)->encoder_ids[(__i)] != 0; (__i)++) \
> > > > + for_each_if((encoder_id) = (connector)->encoder_ids[(__i)])
> > > > +
> > > > #endif
> > > > --
> > > > 2.16.4
> > > >
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
> --
> Ville Syrjälä
> Intel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 6/8] drm/nouveau: Use drm_for_each_connector_encoder_ids()
2018-06-26 17:47 [PATCH 0/8] drm: Second attempt at fixing the fb-helper .best_encoder() mess Ville Syrjala
` (2 preceding siblings ...)
2018-06-26 17:47 ` [PATCH 3/8] drm: Add drm_for_each_connector_encoder_ids() Ville Syrjala
@ 2018-06-26 17:47 ` Ville Syrjala
2018-06-26 20:55 ` [Intel-gfx] " kbuild test robot
[not found] ` <20180626174714.32012-1-ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
` (4 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjala @ 2018-06-26 17:47 UTC (permalink / raw)
To: dri-devel; +Cc: nouveau, intel-gfx, Ben Skeggs
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Use drm_for_each_connector_encoder_ids() for iterating
connector->encoder_ids[]. A bit more convenient not having
to deal with the implementation details.
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: nouveau@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/nouveau/nouveau_connector.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 7b557c354307..ea12fbbea92e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -366,14 +366,11 @@ 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;
+ u32 encoder_id;
+ int i;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
- id = connector->encoder_ids[i];
- if (!id)
- break;
-
- enc = drm_encoder_find(dev, NULL, id);
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ enc = drm_encoder_find(dev, NULL, encoder_id);
if (!enc)
continue;
nv_encoder = nouveau_encoder(enc);
@@ -423,6 +420,7 @@ nouveau_connector_ddc_detect(struct drm_connector *connector)
struct nouveau_encoder *nv_encoder;
struct drm_encoder *encoder;
int i, panel = -ENODEV;
+ u32 encoder_id;
/* eDP panels need powering on by us (if the VBIOS doesn't default it
* to on) before doing any AUX channel transactions. LVDS panel power
@@ -436,12 +434,8 @@ 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);
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ encoder = drm_encoder_find(dev, NULL, encoder_id);
if (!encoder)
continue;
nv_encoder = nouveau_encoder(encoder);
--
2.16.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 6/8] drm/nouveau: Use drm_for_each_connector_encoder_ids()
2018-06-26 17:47 ` [PATCH 6/8] drm/nouveau: Use drm_for_each_connector_encoder_ids() Ville Syrjala
@ 2018-06-26 20:55 ` kbuild test robot
0 siblings, 0 replies; 21+ messages in thread
From: kbuild test robot @ 2018-06-26 20:55 UTC (permalink / raw)
To: Ville Syrjala; +Cc: nouveau, intel-gfx, kbuild-all, dri-devel, Ben Skeggs
[-- Attachment #1: Type: text/plain, Size: 10080 bytes --]
Hi Ville,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm/drm-next]
[also build test WARNING on v4.18-rc2 next-20180626]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Ville-Syrjala/drm-Second-attempt-at-fixing-the-fb-helper-best_encoder-mess/20180627-024018
base: git://people.freedesktop.org/~airlied/linux.git drm-next
config: x86_64-randconfig-x012-201825 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers/gpu//drm/nouveau/nouveau_connector.c: In function 'nouveau_connector_detect':
>> drivers/gpu//drm/nouveau/nouveau_connector.c:606:33: warning: 'nv_encoder' may be used uninitialized in this function [-Wmaybe-uninitialized]
if (nv_partner && ((nv_encoder->dcb->type == DCB_OUTPUT_ANALOG &&
~~~~~~~~~~^~~~~
vim +/nv_encoder +606 drivers/gpu//drm/nouveau/nouveau_connector.c
6ee73861 Ben Skeggs 2009-12-11 546
6ee73861 Ben Skeggs 2009-12-11 547 static enum drm_connector_status
930a9e28 Chris Wilson 2010-09-14 548 nouveau_connector_detect(struct drm_connector *connector, bool force)
6ee73861 Ben Skeggs 2009-12-11 549 {
6ee73861 Ben Skeggs 2009-12-11 550 struct drm_device *dev = connector->dev;
77145f1c Ben Skeggs 2012-07-31 551 struct nouveau_drm *drm = nouveau_drm(dev);
6ee73861 Ben Skeggs 2009-12-11 552 struct nouveau_connector *nv_connector = nouveau_connector(connector);
6ee73861 Ben Skeggs 2009-12-11 553 struct nouveau_encoder *nv_encoder = NULL;
e19b20bb Ben Skeggs 2011-07-12 554 struct nouveau_encoder *nv_partner;
2aa5eac5 Ben Skeggs 2015-08-20 555 struct i2c_adapter *i2c;
03cd06ca Francisco Jerez 2010-07-20 556 int type;
5addcf0a Dave Airlie 2012-09-10 557 int ret;
5addcf0a Dave Airlie 2012-09-10 558 enum drm_connector_status conn_status = connector_status_disconnected;
6ee73861 Ben Skeggs 2009-12-11 559
b8780e2a Francisco Jerez 2010-01-14 560 /* Cleanup the previous EDID block. */
b8780e2a Francisco Jerez 2010-01-14 561 if (nv_connector->edid) {
b8780e2a Francisco Jerez 2010-01-14 562 drm_mode_connector_update_edid_property(connector, NULL);
c8ebe275 Xavier Chantry 2010-01-11 563 kfree(nv_connector->edid);
c8ebe275 Xavier Chantry 2010-01-11 564 nv_connector->edid = NULL;
b8780e2a Francisco Jerez 2010-01-14 565 }
c8ebe275 Xavier Chantry 2010-01-11 566
d61a5c10 Lukas Wunner 2018-02-11 567 /* Outputs are only polled while runtime active, so acquiring a
d61a5c10 Lukas Wunner 2018-02-11 568 * runtime PM ref here is unnecessary (and would deadlock upon
d61a5c10 Lukas Wunner 2018-02-11 569 * runtime suspend because it waits for polling to finish).
d61a5c10 Lukas Wunner 2018-02-11 570 */
d61a5c10 Lukas Wunner 2018-02-11 571 if (!drm_kms_helper_is_poll_worker()) {
5addcf0a Dave Airlie 2012-09-10 572 ret = pm_runtime_get_sync(connector->dev->dev);
b6c4285a Alexandre Courbot 2014-02-12 573 if (ret < 0 && ret != -EACCES)
5addcf0a Dave Airlie 2012-09-10 574 return conn_status;
d61a5c10 Lukas Wunner 2018-02-11 575 }
5addcf0a Dave Airlie 2012-09-10 576
8777c5c1 Ben Skeggs 2014-06-06 577 nv_encoder = nouveau_connector_ddc_detect(connector);
8777c5c1 Ben Skeggs 2014-06-06 578 if (nv_encoder && (i2c = nv_encoder->i2c) != NULL) {
39c1c901 Lukas Wunner 2016-01-11 579 if ((vga_switcheroo_handler_flags() &
39c1c901 Lukas Wunner 2016-01-11 580 VGA_SWITCHEROO_CAN_SWITCH_DDC) &&
39c1c901 Lukas Wunner 2016-01-11 581 nv_connector->type == DCB_CONNECTOR_LVDS)
39c1c901 Lukas Wunner 2016-01-11 582 nv_connector->edid = drm_get_edid_switcheroo(connector,
39c1c901 Lukas Wunner 2016-01-11 583 i2c);
39c1c901 Lukas Wunner 2016-01-11 584 else
2aa5eac5 Ben Skeggs 2015-08-20 585 nv_connector->edid = drm_get_edid(connector, i2c);
39c1c901 Lukas Wunner 2016-01-11 586
6ee73861 Ben Skeggs 2009-12-11 587 drm_mode_connector_update_edid_property(connector,
6ee73861 Ben Skeggs 2009-12-11 588 nv_connector->edid);
6ee73861 Ben Skeggs 2009-12-11 589 if (!nv_connector->edid) {
77145f1c Ben Skeggs 2012-07-31 590 NV_ERROR(drm, "DDC responded, but no EDID for %s\n",
8c6c361a Jani Nikula 2014-06-03 591 connector->name);
0ed3165e Francisco Jerez 2010-01-14 592 goto detect_analog;
6ee73861 Ben Skeggs 2009-12-11 593 }
6ee73861 Ben Skeggs 2009-12-11 594
6ee73861 Ben Skeggs 2009-12-11 595 /* Override encoder type for DVI-I based on whether EDID
6ee73861 Ben Skeggs 2009-12-11 596 * says the display is digital or analog, both use the
6ee73861 Ben Skeggs 2009-12-11 597 * same i2c channel so the value returned from ddc_detect
6ee73861 Ben Skeggs 2009-12-11 598 * isn't necessarily correct.
6ee73861 Ben Skeggs 2009-12-11 599 */
e19b20bb Ben Skeggs 2011-07-12 600 nv_partner = NULL;
cb75d97e Ben Skeggs 2012-07-11 601 if (nv_encoder->dcb->type == DCB_OUTPUT_TMDS)
cb75d97e Ben Skeggs 2012-07-11 602 nv_partner = find_encoder(connector, DCB_OUTPUT_ANALOG);
cb75d97e Ben Skeggs 2012-07-11 603 if (nv_encoder->dcb->type == DCB_OUTPUT_ANALOG)
cb75d97e Ben Skeggs 2012-07-11 604 nv_partner = find_encoder(connector, DCB_OUTPUT_TMDS);
cb75d97e Ben Skeggs 2012-07-11 605
cb75d97e Ben Skeggs 2012-07-11 @606 if (nv_partner && ((nv_encoder->dcb->type == DCB_OUTPUT_ANALOG &&
cb75d97e Ben Skeggs 2012-07-11 607 nv_partner->dcb->type == DCB_OUTPUT_TMDS) ||
cb75d97e Ben Skeggs 2012-07-11 608 (nv_encoder->dcb->type == DCB_OUTPUT_TMDS &&
cb75d97e Ben Skeggs 2012-07-11 609 nv_partner->dcb->type == DCB_OUTPUT_ANALOG))) {
6ee73861 Ben Skeggs 2009-12-11 610 if (nv_connector->edid->input & DRM_EDID_INPUT_DIGITAL)
cb75d97e Ben Skeggs 2012-07-11 611 type = DCB_OUTPUT_TMDS;
6ee73861 Ben Skeggs 2009-12-11 612 else
cb75d97e Ben Skeggs 2012-07-11 613 type = DCB_OUTPUT_ANALOG;
6ee73861 Ben Skeggs 2009-12-11 614
e19b20bb Ben Skeggs 2011-07-12 615 nv_encoder = find_encoder(connector, type);
6ee73861 Ben Skeggs 2009-12-11 616 }
6ee73861 Ben Skeggs 2009-12-11 617
6ee73861 Ben Skeggs 2009-12-11 618 nouveau_connector_set_encoder(connector, nv_encoder);
5addcf0a Dave Airlie 2012-09-10 619 conn_status = connector_status_connected;
5addcf0a Dave Airlie 2012-09-10 620 goto out;
6ee73861 Ben Skeggs 2009-12-11 621 }
6ee73861 Ben Skeggs 2009-12-11 622
c16c5707 Francisco Jerez 2010-09-09 623 nv_encoder = nouveau_connector_of_detect(connector);
c16c5707 Francisco Jerez 2010-09-09 624 if (nv_encoder) {
c16c5707 Francisco Jerez 2010-09-09 625 nouveau_connector_set_encoder(connector, nv_encoder);
5addcf0a Dave Airlie 2012-09-10 626 conn_status = connector_status_connected;
5addcf0a Dave Airlie 2012-09-10 627 goto out;
c16c5707 Francisco Jerez 2010-09-09 628 }
c16c5707 Francisco Jerez 2010-09-09 629
0ed3165e Francisco Jerez 2010-01-14 630 detect_analog:
cb75d97e Ben Skeggs 2012-07-11 631 nv_encoder = find_encoder(connector, DCB_OUTPUT_ANALOG);
f4053509 Ben Skeggs 2010-03-15 632 if (!nv_encoder && !nouveau_tv_disable)
cb75d97e Ben Skeggs 2012-07-11 633 nv_encoder = find_encoder(connector, DCB_OUTPUT_TV);
84b8081c Francisco Jerez 2010-10-26 634 if (nv_encoder && force) {
6ee73861 Ben Skeggs 2009-12-11 635 struct drm_encoder *encoder = to_drm_encoder(nv_encoder);
d58ded76 Jani Nikula 2015-03-11 636 const struct drm_encoder_helper_funcs *helper =
6ee73861 Ben Skeggs 2009-12-11 637 encoder->helper_private;
6ee73861 Ben Skeggs 2009-12-11 638
6ee73861 Ben Skeggs 2009-12-11 639 if (helper->detect(encoder, connector) ==
6ee73861 Ben Skeggs 2009-12-11 640 connector_status_connected) {
6ee73861 Ben Skeggs 2009-12-11 641 nouveau_connector_set_encoder(connector, nv_encoder);
5addcf0a Dave Airlie 2012-09-10 642 conn_status = connector_status_connected;
5addcf0a Dave Airlie 2012-09-10 643 goto out;
6ee73861 Ben Skeggs 2009-12-11 644 }
6ee73861 Ben Skeggs 2009-12-11 645
6ee73861 Ben Skeggs 2009-12-11 646 }
6ee73861 Ben Skeggs 2009-12-11 647
5addcf0a Dave Airlie 2012-09-10 648 out:
5addcf0a Dave Airlie 2012-09-10 649
d61a5c10 Lukas Wunner 2018-02-11 650 if (!drm_kms_helper_is_poll_worker()) {
5addcf0a Dave Airlie 2012-09-10 651 pm_runtime_mark_last_busy(connector->dev->dev);
5addcf0a Dave Airlie 2012-09-10 652 pm_runtime_put_autosuspend(connector->dev->dev);
d61a5c10 Lukas Wunner 2018-02-11 653 }
5addcf0a Dave Airlie 2012-09-10 654
5addcf0a Dave Airlie 2012-09-10 655 return conn_status;
6ee73861 Ben Skeggs 2009-12-11 656 }
6ee73861 Ben Skeggs 2009-12-11 657
:::::: The code at line 606 was first introduced by commit
:::::: cb75d97e9c77743ecfcc43375be135a55a4d9b25 drm/nouveau: implement devinit subdev, and new init table parser
:::::: TO: Ben Skeggs <bskeggs@redhat.com>
:::::: CC: Ben Skeggs <bskeggs@redhat.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27704 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <20180626174714.32012-1-ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* [PATCH 4/8] drm/amdgpu: Use drm_for_each_connector_encoder_ids()
[not found] ` <20180626174714.32012-1-ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2018-06-26 17:47 ` Ville Syrjala
2018-06-26 17:47 ` [PATCH 5/8] drm/msm: " Ville Syrjala
2018-06-26 17:47 ` [PATCH 7/8] drm/radeon: " Ville Syrjala
2 siblings, 0 replies; 21+ messages in thread
From: Ville Syrjala @ 2018-06-26 17:47 UTC (permalink / raw)
To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: David (ChunMing) Zhou, intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Alex Deucher,
Harry Wentland, Christian König
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Use drm_for_each_connector_encoder_ids() for iterating
connector->encoder_ids[]. A bit more convenient not having
to deal with the implementation details.
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>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 51 ++++++++++----------------
drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 15 ++++----
2 files changed, 27 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index 8e66851eb427..dcbe45f6d941 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -216,16 +216,13 @@ amdgpu_connector_update_scratch_regs(struct drm_connector *connector,
struct drm_encoder *encoder = NULL;
const struct drm_connector_helper_funcs *connector_funcs = connector->helper_private;
bool connected;
+ u32 encoder_id;
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]);
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ encoder = drm_encoder_find(connector->dev, NULL, encoder_id);
if (!encoder)
continue;
@@ -244,13 +241,11 @@ amdgpu_connector_find_encoder(struct drm_connector *connector,
int encoder_type)
{
struct drm_encoder *encoder;
+ u32 encoder_id;
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]);
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ encoder = drm_encoder_find(connector->dev, NULL, encoder_id);
if (!encoder)
continue;
@@ -987,7 +982,7 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force)
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,11 +1072,11 @@ 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;
+ u32 encoder_id;
+ int i;
- encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ encoder = drm_encoder_find(connector->dev, NULL, encoder_id);
if (!encoder)
continue;
@@ -1135,12 +1130,11 @@ 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;
+ u32 encoder_id;
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]);
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ encoder = drm_encoder_find(connector->dev, NULL, encoder_id);
if (!encoder)
continue;
@@ -1294,14 +1288,11 @@ u16 amdgpu_connector_encoder_get_dp_bridge_encoder_id(struct drm_connector *conn
{
struct drm_encoder *encoder;
struct amdgpu_encoder *amdgpu_encoder;
+ u32 encoder_id;
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]);
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ encoder = drm_encoder_find(connector->dev, NULL, encoder_id);
if (!encoder)
continue;
@@ -1323,14 +1314,12 @@ static bool amdgpu_connector_encoder_is_hbr2(struct drm_connector *connector)
{
struct drm_encoder *encoder;
struct amdgpu_encoder *amdgpu_encoder;
+ u32 encoder_id;
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]);
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ encoder = drm_encoder_find(connector->dev, NULL, encoder_id);
if (!encoder)
continue;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index dbf2ccd0c744..f8d6318ddbba 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -269,15 +269,12 @@ 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;
+ u32 encoder_id;
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]);
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ encoder = drm_encoder_find(connector->dev, NULL, encoder_id);
if (!encoder)
continue;
@@ -286,8 +283,10 @@ dce_virtual_encoder(struct drm_connector *connector)
}
/* pick the first one */
- if (enc_id)
- return drm_encoder_find(connector->dev, NULL, enc_id);
+ encoder_id = connector->encoder_ids[0];
+ if (encoder_id)
+ return drm_encoder_find(connector->dev, NULL, encoder_id);
+
return NULL;
}
--
2.16.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 5/8] drm/msm: Use drm_for_each_connector_encoder_ids()
[not found] ` <20180626174714.32012-1-ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-06-26 17:47 ` [PATCH 4/8] drm/amdgpu: " Ville Syrjala
@ 2018-06-26 17:47 ` Ville Syrjala
2018-06-26 17:47 ` [PATCH 7/8] drm/radeon: " Ville Syrjala
2 siblings, 0 replies; 21+ messages in thread
From: Ville Syrjala @ 2018-06-26 17:47 UTC (permalink / raw)
To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Rob Clark
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Use drm_for_each_connector_encoder_ids() for iterating
connector->encoder_ids[]. A bit more convenient not having
to deal with the implementation details.
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 | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 4cb1cb68878b..26337be9a1ec 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -751,10 +751,11 @@ 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) {
+ u32 encoder_id;
int i;
- for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
- if (connector->encoder_ids[i] == encoder->base.id)
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ if (encoder_id == encoder->base.id)
return connector;
}
}
--
2.16.4
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 7/8] drm/radeon: Use drm_for_each_connector_encoder_ids()
[not found] ` <20180626174714.32012-1-ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-06-26 17:47 ` [PATCH 4/8] drm/amdgpu: " Ville Syrjala
2018-06-26 17:47 ` [PATCH 5/8] drm/msm: " Ville Syrjala
@ 2018-06-26 17:47 ` Ville Syrjala
2 siblings, 0 replies; 21+ messages in thread
From: Ville Syrjala @ 2018-06-26 17:47 UTC (permalink / raw)
To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: David (ChunMing) Zhou, intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Alex Deucher,
Harry Wentland, Christian König
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Use drm_for_each_connector_encoder_ids() for iterating
connector->encoder_ids[]. A bit more convenient not having
to deal with the implementation details.
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>
---
drivers/gpu/drm/radeon/radeon_connectors.c | 63 ++++++++++++------------------
1 file changed, 26 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 2aea2bdff99b..7fc7e72a075f 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -248,16 +248,13 @@ radeon_connector_update_scratch_regs(struct drm_connector *connector, enum drm_c
struct drm_encoder *encoder = NULL;
const struct drm_connector_helper_funcs *connector_funcs = connector->helper_private;
bool connected;
+ u32 encoder_id;
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]);
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ encoder = drm_encoder_find(connector->dev, NULL, encoder_id);
if (!encoder)
continue;
@@ -277,13 +274,11 @@ 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;
+ u32 encoder_id;
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]);
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ encoder = drm_encoder_find(connector->dev, NULL, encoder_id);
if (!encoder)
continue;
@@ -436,6 +431,7 @@ 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;
+ u32 encoder_id;
int i;
list_for_each_entry(conflict, &dev->mode_config.connector_list, head) {
@@ -443,12 +439,10 @@ radeon_connector_analog_encoder_conflict_solve(struct drm_connector *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_for_each_connector_encoder_ids(conflict, encoder_id, i) {
/* if the IDs match */
- if (conflict->encoder_ids[i] == encoder->base.id) {
+ if (encoder_id == encoder->base.id) {
if (conflict->status != connector_status_connected)
continue;
@@ -1256,7 +1250,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,12 +1368,12 @@ 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;
+ u32 encoder_id;
+ int i;
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
encoder = drm_encoder_find(connector->dev, NULL,
- connector->encoder_ids[i]);
+ encoder_id);
if (!encoder)
continue;
@@ -1458,15 +1452,13 @@ 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;
+ u32 encoder_id;
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]);
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ encoder = drm_encoder_find(connector->dev, NULL, encoder_id);
if (!encoder)
continue;
@@ -1484,8 +1476,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);
+ encoder_id = connector->encoder_ids[0];
+ if (encoder_id)
+ return drm_encoder_find(connector->dev, NULL, encoder_id);
return NULL;
}
@@ -1626,13 +1619,11 @@ u16 radeon_connector_encoder_get_dp_bridge_encoder_id(struct drm_connector *conn
{
struct drm_encoder *encoder;
struct radeon_encoder *radeon_encoder;
+ u32 encoder_id;
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]);
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ encoder = drm_encoder_find(connector->dev, NULL, encoder_id);
if (!encoder)
continue;
@@ -1654,14 +1645,12 @@ static bool radeon_connector_encoder_is_hbr2(struct drm_connector *connector)
{
struct drm_encoder *encoder;
struct radeon_encoder *radeon_encoder;
+ u32 encoder_id;
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]);
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i) {
+ encoder = drm_encoder_find(connector->dev, NULL, encoder_id);
if (!encoder)
continue;
--
2.16.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 8/8] drm/tilcdc: Use drm_for_each_connector_encoder_ids()
2018-06-26 17:47 [PATCH 0/8] drm: Second attempt at fixing the fb-helper .best_encoder() mess Ville Syrjala
` (4 preceding siblings ...)
[not found] ` <20180626174714.32012-1-ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2018-06-26 17:47 ` Ville Syrjala
2018-06-26 18:25 ` ✗ Fi.CI.CHECKPATCH: warning for drm: Second attempt at fixing the fb-helper .best_encoder() mess Patchwork
` (2 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Ville Syrjala @ 2018-06-26 17:47 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Tomi Valkeinen, Jyri Sarha
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Use drm_for_each_connector_encoder_ids() for iterating
connector->encoder_ids[]. A bit more convenient not having
to deal with the implementation details.
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 | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
index d651bdd6597e..ed8af21e75b0 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
@@ -103,11 +103,12 @@ struct drm_connector *tilcdc_encoder_find_connector(struct drm_device *ddev,
struct drm_encoder *encoder)
{
struct drm_connector *connector;
+ u32 encoder_id;
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)
+ drm_for_each_connector_encoder_ids(connector, encoder_id, i)
+ if (encoder_id == encoder->base.id)
return connector;
dev_err(ddev->dev, "No connector found for %s encoder (id %d)\n",
--
2.16.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread* ✗ Fi.CI.CHECKPATCH: warning for drm: Second attempt at fixing the fb-helper .best_encoder() mess
2018-06-26 17:47 [PATCH 0/8] drm: Second attempt at fixing the fb-helper .best_encoder() mess Ville Syrjala
` (5 preceding siblings ...)
2018-06-26 17:47 ` [PATCH 8/8] drm/tilcdc: " Ville Syrjala
@ 2018-06-26 18:25 ` Patchwork
2018-06-26 18:40 ` ✓ Fi.CI.BAT: success " Patchwork
2018-06-26 20:47 ` ✓ Fi.CI.IGT: " Patchwork
8 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2018-06-26 18:25 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: drm: Second attempt at fixing the fb-helper .best_encoder() mess
URL : https://patchwork.freedesktop.org/series/45422/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
eebb696c2139 drm/fb-helper: Eliminate the .best_encoder() usage
b75ca3e84ab2 drm/i915: Nuke intel_mst_best_encoder()
111eeb2a13e6 drm: Add drm_for_each_connector_encoder_ids()
-:131: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'connector' - possible side-effects?
#131: FILE: include/drm/drm_connector.h:1202:
+#define drm_for_each_connector_encoder_ids(connector, encoder_id, __i) \
+ for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
+ (connector)->encoder_ids[(__i)] != 0; (__i)++) \
+ for_each_if((encoder_id) = (connector)->encoder_ids[(__i)])
-:131: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__i' - possible side-effects?
#131: FILE: include/drm/drm_connector.h:1202:
+#define drm_for_each_connector_encoder_ids(connector, encoder_id, __i) \
+ for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
+ (connector)->encoder_ids[(__i)] != 0; (__i)++) \
+ for_each_if((encoder_id) = (connector)->encoder_ids[(__i)])
total: 0 errors, 0 warnings, 2 checks, 97 lines checked
80d66565fb4e drm/amdgpu: Use drm_for_each_connector_encoder_ids()
5665f4e07eb7 drm/msm: Use drm_for_each_connector_encoder_ids()
f1b7ac77b2fc drm/nouveau: Use drm_for_each_connector_encoder_ids()
d955c57fd754 drm/radeon: Use drm_for_each_connector_encoder_ids()
03e3457fbf75 drm/tilcdc: Use drm_for_each_connector_encoder_ids()
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* ✓ Fi.CI.BAT: success for drm: Second attempt at fixing the fb-helper .best_encoder() mess
2018-06-26 17:47 [PATCH 0/8] drm: Second attempt at fixing the fb-helper .best_encoder() mess Ville Syrjala
` (6 preceding siblings ...)
2018-06-26 18:25 ` ✗ Fi.CI.CHECKPATCH: warning for drm: Second attempt at fixing the fb-helper .best_encoder() mess Patchwork
@ 2018-06-26 18:40 ` Patchwork
2018-06-26 20:47 ` ✓ Fi.CI.IGT: " Patchwork
8 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2018-06-26 18:40 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: drm: Second attempt at fixing the fb-helper .best_encoder() mess
URL : https://patchwork.freedesktop.org/series/45422/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4381 -> Patchwork_9428 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/45422/revisions/1/mbox/
== Changes ==
No changes found
== Participating hosts (44 -> 39) ==
Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4381 -> Patchwork_9428
CI_DRM_4381: ed0d219201c3fd3eb430b712d6ceb51b423daefc @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4530: 0e98bf69f146eb72fe3a7c3b19a049b5786f0ca3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9428: 03e3457fbf755417391ae680b5ad5e2f7fced9b3 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
03e3457fbf75 drm/tilcdc: Use drm_for_each_connector_encoder_ids()
d955c57fd754 drm/radeon: Use drm_for_each_connector_encoder_ids()
f1b7ac77b2fc drm/nouveau: Use drm_for_each_connector_encoder_ids()
5665f4e07eb7 drm/msm: Use drm_for_each_connector_encoder_ids()
80d66565fb4e drm/amdgpu: Use drm_for_each_connector_encoder_ids()
111eeb2a13e6 drm: Add drm_for_each_connector_encoder_ids()
b75ca3e84ab2 drm/i915: Nuke intel_mst_best_encoder()
eebb696c2139 drm/fb-helper: Eliminate the .best_encoder() usage
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9428/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* ✓ Fi.CI.IGT: success for drm: Second attempt at fixing the fb-helper .best_encoder() mess
2018-06-26 17:47 [PATCH 0/8] drm: Second attempt at fixing the fb-helper .best_encoder() mess Ville Syrjala
` (7 preceding siblings ...)
2018-06-26 18:40 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-06-26 20:47 ` Patchwork
8 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2018-06-26 20:47 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm: Second attempt at fixing the fb-helper .best_encoder() mess
URL : https://patchwork.freedesktop.org/series/45422/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4381_full -> Patchwork_9428_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_9428_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9428_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9428_full:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_schedule@deep-vebox:
shard-kbl: SKIP -> PASS +2
igt@gem_linear_blits@interruptible:
shard-apl: SKIP -> PASS
igt@gem_mocs_settings@mocs-rc6-bsd2:
shard-kbl: PASS -> SKIP
== Known issues ==
Here are the changes found in Patchwork_9428_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_flip@2x-flip-vs-expired-vblank:
shard-glk: PASS -> FAIL (fdo#105363)
igt@kms_flip@flip-vs-expired-vblank:
shard-glk: PASS -> FAIL (fdo#105189)
igt@kms_flip@plain-flip-fb-recreate:
shard-glk: PASS -> FAIL (fdo#100368)
igt@perf@blocking:
shard-hsw: PASS -> FAIL (fdo#102252)
==== Possible fixes ====
igt@gem_exec_await@wide-contexts:
shard-glk: FAIL (fdo#105900) -> PASS
igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
shard-hsw: FAIL (fdo#105767) -> PASS
igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size:
shard-hsw: FAIL (fdo#103355) -> PASS
igt@kms_flip_tiling@flip-x-tiled:
shard-glk: FAIL (fdo#104724, fdo#103822) -> PASS +2
igt@kms_setmode@basic:
shard-apl: FAIL (fdo#99912) -> PASS
shard-kbl: FAIL (fdo#99912) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4381 -> Patchwork_9428
CI_DRM_4381: ed0d219201c3fd3eb430b712d6ceb51b423daefc @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4530: 0e98bf69f146eb72fe3a7c3b19a049b5786f0ca3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9428: 03e3457fbf755417391ae680b5ad5e2f7fced9b3 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9428/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread