From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751086AbdE3H1k (ORCPT ); Tue, 30 May 2017 03:27:40 -0400 Received: from mail-wm0-f44.google.com ([74.125.82.44]:35471 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750890AbdE3H1h (ORCPT ); Tue, 30 May 2017 03:27:37 -0400 Subject: Re: [PATCH v5 03/10] drm: Use new mode_valid() helpers in connector probe helper To: Jose Abreu , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: Cc: Laurent Pinchart , Carlos Palminha From: Neil Armstrong Organization: Baylibre Message-ID: Date: Tue, 30 May 2017 09:27:24 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/25/2017 04:19 PM, Jose Abreu wrote: > This changes the connector probe helper function to use the new > encoder->mode_valid(), bridge->mode_valid() and crtc->mode_valid() > helper callbacks to validate the modes. > > The new callbacks are optional so the behaviour remains the same > if they are not implemented. If they are, then the code loops > through all the connector's encodersXbridgesXcrtcs and calls the > callback. > > If at least a valid encoderXbridgeXcrtc combination is found which > accepts the mode then the function returns MODE_OK. > > Signed-off-by: Jose Abreu > Reviewed-by: Daniel Vetter > Cc: Carlos Palminha > Cc: Dave Airlie > Cc: Andrzej Hajda > Cc: Laurent Pinchart > > Changes v3->v4: > - Change function name (Laurent) > Changes v2->v3: > - Call also bridge->mode_valid (Daniel) > Changes v1->v2: > - Use new helpers suggested by Ville > - Change documentation (Daniel) > > --- > drivers/gpu/drm/drm_probe_helper.c | 67 +++++++++++++++++++++++++++++++++++--- > 1 file changed, 63 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > index f01abdc..00e6832 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -83,6 +83,61 @@ > return MODE_OK; > } > > +static enum drm_mode_status > +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; > + > + /* Step 1: Validate against connector */ > + ret = drm_connector_mode_valid(connector, mode); > + if (ret != MODE_OK) > + 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, ids[i]); > + struct drm_crtc *crtc; > + > + if (!encoder) > + continue; > + > + ret = drm_encoder_mode_valid(encoder, mode); > + if (ret != MODE_OK) { > + /* No point in continuing for crtc check as this encoder > + * will not accept the mode anyway. If all encoders > + * reject the mode then, at exit, ret will not be > + * MODE_OK. */ > + continue; > + } > + > + ret = drm_bridge_mode_valid(encoder->bridge, mode); > + if (ret != MODE_OK) { > + /* There is also no point in continuing for crtc check > + * here. */ > + continue; > + } > + > + drm_for_each_crtc(crtc, dev) { > + if (!drm_encoder_crtc_ok(encoder, crtc)) > + continue; > + > + ret = drm_crtc_mode_valid(crtc, mode); > + if (ret == MODE_OK) { > + /* If we get to this point there is at least > + * one combination of encoder+crtc that works > + * for this mode. Lets return now. */ > + return ret; > + } > + } > + } > + > + return ret; > +} > + > static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector) > { > struct drm_cmdline_mode *cmdline_mode; > @@ -322,7 +377,11 @@ void drm_kms_helper_poll_enable(struct drm_device *dev) > * - drm_mode_validate_flag() checks the modes against basic connector > * capabilities (interlace_allowed,doublescan_allowed,stereo_allowed) > * - the optional &drm_connector_helper_funcs.mode_valid helper can perform > - * driver and/or hardware specific checks > + * driver and/or sink specific checks > + * - the optional &drm_crtc_helper_funcs.mode_valid, > + * &drm_bridge_funcs.mode_valid and &drm_encoder_helper_funcs.mode_valid > + * helpers can perform driver and/or source specific checks which are also > + * enforced by the modeset/atomic helpers > * > * 5. Any mode whose status is not OK is pruned from the connector's modes list, > * accompanied by a debug message indicating the reason for the mode's > @@ -466,9 +525,9 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, > if (mode->status == MODE_OK) > mode->status = drm_mode_validate_flag(mode, mode_flags); > > - if (mode->status == MODE_OK && connector_funcs->mode_valid) > - mode->status = connector_funcs->mode_valid(connector, > - mode); > + if (mode->status == MODE_OK) > + mode->status = drm_mode_validate_pipeline(mode, > + connector); > } > > prune: > Reviewed-by: Neil Armstrong