From patchwork Fri Apr 21 13:29:12 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jose Abreu X-Patchwork-Id: 9694177 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id CFEBE6038D for ; Sat, 22 Apr 2017 06:08:13 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BFD6F28174 for ; Sat, 22 Apr 2017 06:08:13 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B4C3328627; Sat, 22 Apr 2017 06:08:13 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 7BB2D28174 for ; Sat, 22 Apr 2017 06:08:12 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3BE9B6E51E; Sat, 22 Apr 2017 06:07:07 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from smtprelay.synopsys.com (smtprelay2.synopsys.com [198.182.60.111]) by gabe.freedesktop.org (Postfix) with ESMTPS id 532136E51B for ; Fri, 21 Apr 2017 13:29:16 +0000 (UTC) Received: from mailhost.synopsys.com (mailhost2.synopsys.com [10.13.184.66]) by smtprelay.synopsys.com (Postfix) with ESMTP id 832B510C155E; Fri, 21 Apr 2017 06:29:15 -0700 (PDT) Received: from mailhost.synopsys.com (localhost [127.0.0.1]) by mailhost.synopsys.com (Postfix) with ESMTP id 457171E0; Fri, 21 Apr 2017 06:29:15 -0700 (PDT) Received: from us01wehtc1.internal.synopsys.com (us01wehtc1.internal.synopsys.com [10.12.239.235]) by mailhost.synopsys.com (Postfix) with ESMTP id DA4FD1D9; Fri, 21 Apr 2017 06:29:14 -0700 (PDT) Received: from DE02WEHTCB.internal.synopsys.com (10.225.19.94) by us01wehtc1.internal.synopsys.com (10.12.239.235) with Microsoft SMTP Server (TLS) id 14.3.266.1; Fri, 21 Apr 2017 06:29:14 -0700 Received: from DE02WEHTCA.internal.synopsys.com (10.225.19.92) by DE02WEHTCB.internal.synopsys.com (10.225.19.94) with Microsoft SMTP Server (TLS) id 14.3.266.1; Fri, 21 Apr 2017 15:29:13 +0200 Received: from [10.0.2.15] (10.107.19.99) by DE02WEHTCA.internal.synopsys.com (10.225.19.80) with Microsoft SMTP Server (TLS) id 14.3.266.1; Fri, 21 Apr 2017 15:29:13 +0200 Subject: Re: Probing modes and validating them To: "dri-devel@lists.freedesktop.org" , Daniel Vetter , Dave Airlie References: <243e057b-dac0-7e70-7f15-e469e3f77a2a@synopsys.com> From: Jose Abreu Message-ID: Date: Fri, 21 Apr 2017 14:29:12 +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: <243e057b-dac0-7e70-7f15-e469e3f77a2a@synopsys.com> X-Originating-IP: [10.107.19.99] X-Mailman-Approved-At: Sat, 22 Apr 2017 06:06:56 +0000 Cc: Alexey Brodkin X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP ++ Daniel ++ Dave On 21-04-2017 13:59, Jose Abreu wrote: > Hi All, > > > Is there any callback available, at the crtc level, that can > validate the probed modes before making them available for > userspace? I mean: I know that there is connector->mode_valid(), > but I couldn't find any equivalent crtc->mode_valid(). I found > mode_fixup() but this is called after giving the mode to > userspace, which is not what I need. > > For reference here is the situation I'm facing: > > ARCPGU is a DRM driver that uses ADV7511 as connector/bridge. The > PGU encodes raw video into a stream that ADV can understands and > in order to do this it needs a pixel clock. Currently this pixel > clock value is fixed, so technically arcpgu driver supports only > one video mode. There is a patch currently on discussion that > adds more supported frequencies for arcpgu, but there are still > some frequencies that will not be supported. This is NOT a > limitation in ADV7511, so it doesn't make sense to limit the > available modes in adv, we could instead limit the modes in the > crtc level (i.e. the pgu). > > In order to know if a mode can be displayed or not it is as > simple as call clk_round_rate() and check if the returned > frequency is the same. But in order to do this I need some sort > of callback that is called at the crtc level and before > delivering modes to userspace. > > I believe this would be a good addition to arcpgu because this > way we wouldn't "fool" userspace by delivering modes that will > fail in atomic check. The user may still specify manually a mode, > this will still fail in atomic check, but the difference is that > this mode will not be in the probed list. > > > Best regards, > > Jose Miguel Abreu > With this simple patch I could fix my problem, what do you think? Is this ok to be added? I guess some connectors may not have the crtc linked at probbing stage but this is handled. From 252b7cb5ad9999eaf16be95988d17468eea2895b Mon Sep 17 00:00:00 2001 Message-Id: <252b7cb5ad9999eaf16be95988d17468eea2895b.1492781127.git.joabreu@synopsys.com> From: Jose Abreu Date: Fri, 21 Apr 2017 14:24:16 +0100 Subject: [PATCH] drm: Introduce crtc->mode_valid() callback Introduce a new crtc callback which validates the probbed modes, just like connector->mode_valid() does. Signed-off-by: Jose Abreu --- drivers/gpu/drm/arc/arcpgu_crtc.c | 14 ++++++++++++++ drivers/gpu/drm/drm_probe_helper.c | 12 ++++++++++++ include/drm/drm_modeset_helper_vtables.h | 3 +++ 3 files changed, 29 insertions(+) diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c index 7130b04..e43e8d6 100644 --- a/drivers/gpu/drm/arc/arcpgu_crtc.c +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c @@ -63,6 +63,19 @@ static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc) .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, }; +enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc, + struct drm_display_mode *mode) +{ + struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); + long rate, clk_rate = mode->clock * 1000; + + rate = clk_round_rate(arcpgu->clk, clk_rate); + if (rate != clk_rate) + return MODE_NOCLOCK; + + return MODE_OK; +} + static void arc_pgu_crtc_mode_set_nofb(struct drm_crtc *crtc) { struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); @@ -157,6 +170,7 @@ static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc, } static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = { + .mode_valid = arc_pgu_crtc_mode_valid, .mode_set = drm_helper_crtc_mode_set, .mode_set_base = drm_helper_crtc_mode_set_base, .mode_set_nofb = arc_pgu_crtc_mode_set_nofb, diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index cf8f012..6c9af88 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -233,6 +233,8 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, struct drm_display_mode *mode; const struct drm_connector_helper_funcs *connector_funcs = connector->helper_private; + struct drm_crtc *crtc = NULL; + const struct drm_crtc_helper_funcs *crtc_funcs = NULL; int count = 0; int mode_flags = 0; bool verbose_prune = true; @@ -242,6 +244,13 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, connector->name); + + if (connector->encoder && connector->encoder->crtc && + connector->encoder->crtc->helper_private) { + crtc = connector->encoder->crtc; + crtc_funcs = crtc->helper_private; + } + /* set all old modes to the stale state */ list_for_each_entry(mode, &connector->modes, head) mode->status = MODE_STALE; @@ -338,6 +347,9 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, if (mode->status == MODE_OK && connector_funcs->mode_valid) mode->status = connector_funcs->mode_valid(connector, mode); + + if (mode->status == MODE_OK && crtc && crtc_funcs->mode_valid) + mode->status = crtc_funcs->mode_valid(crtc, mode); } prune: diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index 69c3974..776c9d0 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -104,6 +104,9 @@ struct drm_crtc_helper_funcs { */ void (*commit)(struct drm_crtc *crtc); + enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc, + struct drm_display_mode *mode); + /** * @mode_fixup: *