From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753226AbdEJI5i (ORCPT ); Wed, 10 May 2017 04:57:38 -0400 Received: from smtprelay4.synopsys.com ([198.182.47.9]:51213 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753008AbdEJI5f (ORCPT ); Wed, 10 May 2017 04:57:35 -0400 Subject: Re: [PATCH v2 2/8] drm: Add drm_crtc_mode_valid() To: Daniel Vetter References: <9fa7d9826e3793989fb52a2911d37b41ddfc0580.1494347165.git.joabreu@synopsys.com> <20170510075906.h67prtmveosrmgb2@phenom.ffwll.local> From: Jose Abreu CC: Jose Abreu , , , Carlos Palminha , Alexey Brodkin , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Dave Airlie , Andrzej Hajda , Archit Taneja Message-ID: <8af70047-7ab8-9beb-5258-ebaec5a245d8@synopsys.com> Date: Wed, 10 May 2017 09:57:30 +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: <20170510075906.h67prtmveosrmgb2@phenom.ffwll.local> 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 Daniel, On 10-05-2017 08:59, Daniel Vetter wrote: > On Tue, May 09, 2017 at 06:00:09PM +0100, Jose Abreu wrote: >> Add a new helper to call crtc->mode_valid callback. >> >> Suggested-by: Ville Syrjälä >> 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_crtc.c | 22 ++++++++++++++++++++++ >> drivers/gpu/drm/drm_crtc_internal.h | 3 +++ >> 2 files changed, 25 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c >> index 5af25ce..07ae705 100644 >> --- a/drivers/gpu/drm/drm_crtc.c >> +++ b/drivers/gpu/drm/drm_crtc.c >> @@ -38,6 +38,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -741,3 +742,24 @@ int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj, >> >> return ret; >> } >> + >> +/** >> + * drm_crtc_mode_valid - call crtc->mode_valid callback, if any. >> + * @crtc: crtc >> + * @mode: mode to be validated >> + * >> + * If no mode_valid callback is available this will return MODE_OK. >> + * >> + * Returns: drm_mode_status Enum >> + */ >> +enum drm_mode_status drm_crtc_mode_valid(struct drm_crtc *crtc, >> + const struct drm_display_mode *mode) >> +{ >> + const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private; > This is clearly a helper func, but you place it into the core and > EXPORT_SYMBOL it. Imo this should be entirely internal to the helpers, > perhaps just stuff them all into drm_probe_helpers.c? Header file would be > drm_crtc_helper_internal.h. Yeah, at first I was not planning to export it but then I saw that drm_bridge_mode_fixup() is exported (and is in drm_bridge.c) so it kind of felt right to place this in drm_crtc.c. Anyway, I will move them to drm_probe_helpers.c, indeed there is no point in exporting this. > > That also means no need for kernel-doc (only the driver api is formally > documented) and then these 3 patches are so tiny it's better to squash > them into the patch that adds their users. Ok, will remove the docs but I think its better to have a single patch which adds all the helpers so that I can use the suggested-by tag. Thanks! Best regards, Jose Miguel Abreu > > Thanks, Daniel >> + >> + if (!crtc_funcs || !crtc_funcs->mode_valid) >> + return MODE_OK; >> + >> + return crtc_funcs->mode_valid(crtc, mode); >> +} >> +EXPORT_SYMBOL(drm_crtc_mode_valid); >> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h >> index d077c54..3800abd 100644 >> --- a/drivers/gpu/drm/drm_crtc_internal.h >> +++ b/drivers/gpu/drm/drm_crtc_internal.h >> @@ -45,6 +45,9 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc, >> >> struct dma_fence *drm_crtc_create_fence(struct drm_crtc *crtc); >> >> +enum drm_mode_status drm_crtc_mode_valid(struct drm_crtc *crtc, >> + const struct drm_display_mode *mode); >> + >> /* IOCTLs */ >> int drm_mode_getcrtc(struct drm_device *dev, >> void *data, struct drm_file *file_priv); >> -- >> 1.9.1 >> >>