From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753371AbdEJOCf (ORCPT ); Wed, 10 May 2017 10:02:35 -0400 Received: from us01smtprelay-2.synopsys.com ([198.182.60.111]:41294 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751267AbdEJOCe (ORCPT ); Wed, 10 May 2017 10:02:34 -0400 Subject: Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid() To: =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , "Jose Abreu" References: <20170510134109.GF12629@intel.com> CC: , , Carlos Palminha , Alexey Brodkin , Daniel Vetter , "Dave Airlie" , Andrzej Hajda , "Archit Taneja" From: Jose Abreu Message-ID: <373cc5de-5ce8-5786-dd1a-7ab7bd09d4bc@synopsys.com> Date: Wed, 10 May 2017 15:01:35 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20170510134109.GF12629@intel.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.107.19.62] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Ville, On 10-05-2017 14:41, Ville Syrjälä wrote: > On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote: >> Introduce a new helper function which calls mode_valid() callback >> for all bridges in an encoder chain. >> >> 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_bridge.c | 33 +++++++++++++++++++++++++++++++++ >> include/drm/drm_bridge.h | 2 ++ >> 2 files changed, 35 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c >> index 86a7637..dc8cdfe 100644 >> --- a/drivers/gpu/drm/drm_bridge.c >> +++ b/drivers/gpu/drm/drm_bridge.c >> @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge, >> EXPORT_SYMBOL(drm_bridge_mode_fixup); >> >> /** >> + * drm_bridge_mode_valid - validate the mode against all bridges in the >> + * encoder chain. >> + * @bridge: bridge control structure >> + * @mode: desired mode to be validated >> + * >> + * Calls &drm_bridge_funcs.mode_valid for all the bridges in the encoder >> + * chain, starting from the first bridge to the last. If at least one bridge >> + * does not accept the mode the function returns the error code. >> + * >> + * Note: the bridge passed should be the one closest to the encoder. >> + * >> + * RETURNS: >> + * MODE_OK on success, drm_mode_status Enum error code on failure >> + */ >> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge, >> + const struct drm_display_mode *mode) >> +{ >> + enum drm_mode_status ret = MODE_OK; >> + >> + if (!bridge) >> + return ret; >> + >> + if (bridge->funcs->mode_valid) >> + ret = bridge->funcs->mode_valid(bridge, mode); >> + >> + if (ret != MODE_OK) >> + return ret; >> + >> + return drm_bridge_mode_valid(bridge->next, mode); > Looks like it should be pretty trivial to avoid the recursion. > > Am I correct in interpreting this that bridges have some kind of > a hand rolled linked list implementation? Reusing the standard > linked lists would allow you to use list_for_each() etc. I reused the drm_bridge_mode_fixup but now I see how its done like that: so that the fixup is propagated in the correct order. As for mode_valid we just need to check if ret != MODE_OK then I think we can use the list_for_each_entry(bridge->list). Best regards, Jose Miguel Abreu > >> +} >> +EXPORT_SYMBOL(drm_bridge_mode_valid); >> + >> +/** >> * drm_bridge_disable - disables all bridges in the encoder chain >> * @bridge: bridge control structure >> * >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h >> index 00c6c36..8358eb3 100644 >> --- a/include/drm/drm_bridge.h >> +++ b/include/drm/drm_bridge.h >> @@ -233,6 +233,8 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge, >> bool drm_bridge_mode_fixup(struct drm_bridge *bridge, >> const struct drm_display_mode *mode, >> struct drm_display_mode *adjusted_mode); >> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge, >> + const struct drm_display_mode *mode); >> void drm_bridge_disable(struct drm_bridge *bridge); >> void drm_bridge_post_disable(struct drm_bridge *bridge); >> void drm_bridge_mode_set(struct drm_bridge *bridge, >> -- >> 1.9.1 >>