Re: [Intel-gfx] [PATCH 1/8] drm/fb-helper: Eliminate the .best_encoder() usage
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ä > > > > 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 > > Cc: Harry Wentland > > Cc: Daniel Vetter > > Acked-by: Daniel Vetter > > Suggested-by: Daniel Vetter > > Signed-off-by: Ville Syrjälä > > --- > > 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 > > > + 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 = _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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/8] drm/fb-helper: Eliminate the .best_encoder() usage
On Tue, Jun 26, 2018 at 08:47:07PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > 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 > Cc: Harry Wentland > Cc: Daniel Vetter > Acked-by: Daniel Vetter > Suggested-by: Daniel Vetter > Signed-off-by: Ville Syrjälä > --- > 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 > + 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 = _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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/8] drm/fb-helper: Eliminate the .best_encoder() usage
On Tue, Jun 26, 2018 at 1:47 PM, Ville Syrjala wrote: > From: Ville Syrjälä > > 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 > Cc: Harry Wentland > Cc: Daniel Vetter > Acked-by: Daniel Vetter > Suggested-by: Daniel Vetter > Signed-off-by: Ville Syrjälä Series is: Reviewed-by: Alex Deucher > --- > 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 = _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-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/8] drm/fb-helper: Eliminate the .best_encoder() usage
From: Ville Syrjälä 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 Cc: Harry Wentland Cc: Daniel Vetter Acked-by: Daniel Vetter Suggested-by: Daniel Vetter Signed-off-by: Ville Syrjälä --- 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 = _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