From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755743AbdEDOdG (ORCPT ); Thu, 4 May 2017 10:33:06 -0400 Received: from mga04.intel.com ([192.55.52.120]:18125 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754645AbdEDOcF (ORCPT ); Thu, 4 May 2017 10:32:05 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,287,1491289200"; d="scan'208";a="1143605870" Date: Thu, 4 May 2017 17:32:00 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Jose Abreu Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Carlos Palminha , Alexey Brodkin , Daniel Vetter , Dave Airlie , Andrzej Hajda , Archit Taneja Subject: Re: [PATCH 2/5] drm: Use new mode_valid() helpers in connector probe helper Message-ID: <20170504143200.GX12629@intel.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 04, 2017 at 03:11:39PM +0100, 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 > --- > drivers/gpu/drm/drm_probe_helper.c | 71 +++++++++++++++++++++++++++++++++++--- > 1 file changed, 67 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > index 1b0c14a..bf19f82 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -80,6 +80,67 @@ > return MODE_OK; > } > > +static enum drm_mode_status > +drm_mode_validate_connector(struct drm_connector *connector, > + struct drm_display_mode *mode) > +{ > + const struct drm_connector_helper_funcs *connector_funcs = > + connector->helper_private; > + 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 */ > + if (connector_funcs && connector_funcs->mode_valid) > + ret = connector_funcs->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]); > + const struct drm_encoder_helper_funcs *encoder_funcs; > + struct drm_crtc *crtc; > + > + if (!encoder) > + continue; > + > + encoder_funcs = encoder->helper_private; > + if (encoder_funcs && encoder_funcs->mode_valid) > + ret = encoder_funcs->mode_valid(encoder, mode); > + else > + ret = MODE_OK; /* encoder accepts everything */ Since you already added the drm_bridge_mode_valid() helper, maybe add one for connectors, encoders and crtcs as well. Might keep the logic in this function a bit more clear on account of not having to check for the presence of the vfunc. Right now all three cases look slightly different from one another. > + > + /* 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. */ > + if (ret != MODE_OK) > + continue; > + > + drm_for_each_crtc(crtc, dev) { > + const struct drm_crtc_helper_funcs *crtc_funcs; > + > + if (!drm_encoder_crtc_ok(encoder, crtc)) > + continue; > + > + crtc_funcs = crtc->helper_private; > + if (!crtc_funcs || !crtc_funcs->mode_valid) > + return MODE_OK; /* crtc accepts everything */ > + > + ret = crtc_funcs->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; > @@ -283,8 +344,10 @@ void drm_kms_helper_poll_enable(struct drm_device *dev) > * (if specified) > * - 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 > + * - the optional &drm_connector_helper_funcs.mode_valid, > + * &drm_crtc_helper_funcs.mode_valid and > + * &drm_encoder_helper_funcs.mode_valid helpers can perform driver and/or > + * hardware specific checks > * > * 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 +491,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); > } > > -- > 1.9.1 > -- Ville Syrjälä Intel OTC