From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756580AbdELJhI (ORCPT ); Fri, 12 May 2017 05:37:08 -0400 Received: from galahad.ideasonboard.com ([185.26.127.97]:36382 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756298AbdELJf2 (ORCPT ); Fri, 12 May 2017 05:35:28 -0400 From: Laurent Pinchart To: dri-devel@lists.freedesktop.org Cc: Jose Abreu , linux-kernel@vger.kernel.org, Daniel Vetter , Alexey Brodkin , Carlos Palminha Subject: Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper Date: Fri, 12 May 2017 12:35:28 +0300 Message-ID: <3844793.Y2ncMspPzi@avalon> User-Agent: KMail/4.14.10 (Linux/4.9.16-gentoo; KDE/4.14.29; x86_64; ; ) In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v4C9co9V026182 Hi Jose, Thank you for the patch. On Tuesday 09 May 2017 18:00:12 Jose Abreu wrote: > This changes the connector probe helper function to use the new > encoder->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 encodersXcrtcs and calls the > callback. > > If at least a valid encoderXcrtc combination is found which > accepts the mode then the function returns MODE_OK. > > Signed-off-by: Jose Abreu > Cc: Carlos Palminha > Cc: Alexey Brodkin > Cc: Ville Syrjälä > Cc: Daniel Vetter > Cc: Dave Airlie > Cc: Andrzej Hajda > Cc: Archit Taneja > --- > > Changes v1->v2: > - Use new helpers suggested by Ville > - Change documentation (Daniel) > > drivers/gpu/drm/drm_probe_helper.c | 60 +++++++++++++++++++++++++++++++++-- > 1 file changed, 57 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_probe_helper.c > b/drivers/gpu/drm/drm_probe_helper.c index 1b0c14a..de47413 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -39,6 +39,8 @@ > #include > #include > > +#include "drm_crtc_internal.h" > + > /** > * DOC: output probing helper overview > * > @@ -80,6 +82,54 @@ > return MODE_OK; > } > > +static enum drm_mode_status > +drm_mode_validate_connector(struct drm_connector *connector, > + struct drm_display_mode *mode) This does more than validating the mode against the connector, it validates it against the whole pipeline. I would call the function drm_mode_validate_pipeline() (or any other similar name). > +{ > + 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; > + } > + > + 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; > @@ -284,7 +334,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 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 > @@ -428,8 +482,8 @@ 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, > + if (mode->status == MODE_OK) > + mode->status = drm_mode_validate_connector(connector, > mode); I would reverse the arguments order to match the style of the other validation functions. > } -- Regards, Laurent Pinchart