From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933438AbdEOIjz (ORCPT ); Mon, 15 May 2017 04:39:55 -0400 Received: from mailout3.w1.samsung.com ([210.118.77.13]:16795 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933310AbdEOIju (ORCPT ); Mon, 15 May 2017 04:39:50 -0400 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 X-AuditID: cbfec7f2-f797e6d000004438-f0-59196949549d Subject: Re: [PATCH v3 4/6] drm: Use new mode_valid() helpers in connector probe helper To: Jose Abreu , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Cc: Carlos Palminha , Alexey Brodkin , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Daniel Vetter , Dave Airlie , Archit Taneja From: Andrzej Hajda Message-id: Date: Mon, 15 May 2017 10:39:35 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 In-reply-to: <46c300b3827190dbb066f1af7b9d32088f8b5542.1494492646.git.joabreu@synopsys.com> Content-transfer-encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrGKsWRmVeSWpSXmKPExsWy7djP87qemZKRBs2nhS16z51kslj39TaT RVPHW1aLWfseslosfHiX2eLK1/dsFvf+fGC1uLxrDpvF938LmRw4PS739TJ57P22gMVj+7cH rB7zTgZ63O8+zuSxZf9nRo/Pm+QC2KO4bFJSczLLUov07RK4Mm4c/8lcsFKj4ujHD2wNjH/l uxg5OSQETCRuNPcyQdhiEhfurWfrYuTiEBJYyijxvvEPM4TzmVHi5oXDQA4HWMfr/1EQ8WWM Eh0LGsC6eQUEJX5MvscCYjMLaEq8+DKJBaLoGaPEqXdfwIqEBSIlTj1fzQxiiwhkSvx//Jsd pIhZYAaTxM2ug+wgCTag7r+bb7JBTLWT6JvcDWazCKhKbHu+hBHEFhWIkLg+ZwuYzSkQK/F7 /1EmiM3yEgevPAfbLCFwil1iQ/MGVoizZSU2HWCG+NNFYtfqDihbWOLV8S3sELaMRGfHQSaI 3m5GiU/9J9ghnCmMEv8+zIDqsJY4fPwiK8Q2PolJ26ZDw4VXoqNNCKLEQ+LDyb+sELajxO8b H6BBcYdR4k37TcYJjPKzkIJsFlKQzULyxAJG5lWMIqmlxbnpqcXGesWJucWleel6yfm5mxiB qef0v+OfdjB+PWF1iFGAg1GJhzfhskSkEGtiWXFl7iFGCQ5mJRHeBVGSkUK8KYmVValF+fFF pTmpxYcYpTlYlMR5uU5dixASSE8sSc1OTS1ILYLJMnFwSjUwrnq1qqRA9vyKoM9yy0OMT/0U qtk/56XKpqWb5xzaxJ+56OrOf5wynHqzVoXdzTKKeFNY/uOw3nuFZ3PXya+cNfeWbkW/ou69 lRlGbzJ1heum8pyyD+vlm/B0+5rUGy2b+J57ZqTHP1hU+Xblhus8E29rHz/gMPlDvMZvnmmu Xy97X4zYPGOuiK4SS3FGoqEWc1FxIgAiUr+YOQMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrEIsWRmVeSWpSXmKPExsVy+t/xa7r3MiUjDXY1iVn0njvJZLHu620m i6aOt6wWs/Y9ZLVY+PAus8WVr+/ZLO79+cBqcXnXHDaL7/8WMjlwelzu62Xy2PttAYvH9m8P WD3mnQz0uN99nMljy/7PjB6fN8kFsEe52WSkJqakFimk5iXnp2TmpdsqhYa46VooKeQl5qba KkXo+oYEKSmUJeaUAnlGBmjAwTnAPVhJ3y7BLePG8Z/MBSs1Ko5+/MDWwPhXvouRg0NCwETi 9f+oLkZOIFNM4sK99WxdjFwcQgJLGCW6/8xmBknwCghK/Jh8jwWknllAXWLKlFyImmeMElue TGICqREWiJQ49Xw1WL2IQKZE05fLrCC2kMAdRokdf8AamAVmMUls2fcWrIFNQFPi7+abbBAL 7CT6JneD2SwCqhLbni9hBLFFBSIkHnbuYgexOQViJU5v3MoCYjMLyEscvPKcZQKjwCwk981C uG8WkqoFjMyrGEVSS4tz03OLDfWKE3OLS/PS9ZLzczcxAuNv27Gfm3cwXtoYfIhRgINRiYd3 x1WJSCHWxLLiytxDjBIczEoivL8zJCOFeFMSK6tSi/Lji0pzUosPMZoC3TqRWUo0OR+YGvJK 4g1NDM0tDY2MLSzMjYyUxHlLPlwJFxJITyxJzU5NLUgtgulj4uCUamDUOOjQ9ahKgfEgT2jt pm0tbgeZZ/25cmP6xHP7nms2lD+qObpjw8qLU9d1Fr3+FjvpT9uC0A7zqN3RnE7WvI+TJh0S mMH16svM68+m7E18fYCrUf/25ERvhXNf9QMuqVnEBi6Me3z93Ta750IenrO+Pv3I+9Rn6SQb k82VQhe1lboNxZXDEhqmKrEUZyQaajEXFScCAJY4ISLVAgAA X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170515083936eucas1p17968ba867f3fded8b85d5c85c2ce40d5 X-Msg-Generator: CA X-Sender-IP: 182.198.249.179 X-Local-Sender: =?UTF-8?B?QW5kcnplaiBIYWpkYRtTUlBPTC1LZXJuZWwgKFRQKRvsgrw=?= =?UTF-8?B?7ISx7KCE7J6QG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Global-Sender: =?UTF-8?B?QW5kcnplaiBIYWpkYRtTUlBPTC1LZXJuZWwgKFRQKRtTYW1z?= =?UTF-8?B?dW5nIEVsZWN0cm9uaWNzG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjczOTI=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20170511090650epcas5p4f40af09dff02831285d0bcef6e12451c X-RootMTR: 20170511090650epcas5p4f40af09dff02831285d0bcef6e12451c References: <46c300b3827190dbb066f1af7b9d32088f8b5542.1494492646.git.joabreu@synopsys.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11.05.2017 11:06, 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 > Cc: Carlos Palminha > Cc: Alexey Brodkin > Cc: Ville Syrjälä > Cc: Daniel Vetter > Cc: Dave Airlie > Cc: Andrzej Hajda > Cc: Archit Taneja > --- > > 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 | 65 ++++++++++++++++++++++++++++++++++++-- > 1 file changed, 62 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > index f01abdc..84d660e 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_connector(struct drm_connector *connector, > + struct drm_display_mode *mode) > +{ > + 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; > + } Maybe it is a bikeshedding, but wouldn't be better to call drm_bridge_mode_valid from drm_encoder_mode_valid, in general call all bridge related stuff from corresponding encoder stuff? This is more question about role of encoder->bridge, should it be treated as encoder's extension, or as 1st class citizen in drm? Another concern is about order of calls, it is from sink to source, to keep it consistent bridge should be called before encoder, am I right? Beside this: Reviewed-by: Andrzej Hajda -- Regards Andrzej > + > + 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,8 +525,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); > } >