| Message ID | 20180626174714.32012-2-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 >
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
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; }