From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757329AbdELNhL (ORCPT ); Fri, 12 May 2017 09:37:11 -0400 Received: from mailout2.w1.samsung.com ([210.118.77.12]:44305 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752996AbdELNhJ (ORCPT ); Fri, 12 May 2017 09:37:09 -0400 MIME-version: 1.0 Content-type: text/plain; charset=windows-1252 X-AuditID: cbfec7ef-f796a6d00000373c-3b-5915ba834e52 Subject: Re: [PATCH v3 0/6] Introduce new mode validation callbacks To: Jose Abreu , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Carlos Palminha , Alexey Brodkin , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Dave Airlie , Archit Taneja From: Andrzej Hajda Message-id: <1b3f1b8a-e3b5-f3e9-4697-bd40bd0cfede@samsung.com> Date: Fri, 12 May 2017 15:37:03 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 In-reply-to: <20170512073232.yxknr556dal6isoh@phenom.ffwll.local> Content-transfer-encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprIKsWRmVeSWpSXmKPExsWy7djP87rNu0QjDfb9srToPXeSyWLd19tM Fk0db1ktZu17yGpx5et7Not7fz6wWlzeNYfN4vu/hUwOHB6X+3qZPLZ/e8DqMe9koMf97uNM Hlv2f2b0+LxJLoAtissmJTUnsyy1SN8ugSvj3+1prAWPZSuOXJ/L0sA4WaKLkZNDQsBE4sKZ tSwQtpjEhXvr2boYuTiEBJYxSrxdvYgdwvnMKNFw9DsrTMfOH+9Y4Kqe/PwAluAVEJT4Mfke 2ChmAQOJGVMOM0EUPWOUuPDkJFiRsICzRMeP2cwgCRGBm0wSx16+ZwZJsAloSvzdfJMNYpKd xKSlN9hBbBYBVYn7i9eDxUUFIiSuz9nCCGJzCjhKXN/dxAixTV7i4JXnYCdJCKxil7j9+j7Q UA4gR1Zi0wFmiLNdJD5Mn8IGYQtLvDq+hR3ClpG4PLkbqrebUeJT/wl2CGcKo8S/DzOguq0l Dh+/yAqxjU9i0rbpUAt4JTrahCBKPCRa7h2CKneU2LJuFTSMDjFKtGw8xj6BUW4WUjDNQgqm WUieWMDIvIpRJLW0ODc9tdhQrzgxt7g0L10vOT93EyMwoZz+d/z9DsanzSGHGAU4GJV4eBXW ikYKsSaWFVfmAi3iYFYS4WXbCRTiTUmsrEotyo8vKs1JLT7EKM3BoiTOy3vqWoSQQHpiSWp2 ampBahFMlomDU6qB0ZLzwW/t7T6zInUqn+nxh1sJ/BXd/1no8FH/b+fWxAZfk9V/fjBAaFno 9xV3fxlN9KiW2eq96F/x1qrHoRf8GmsnxTfbXvkWZHBEqSnMqjLtGOuVLXu//rCq3Rs58c3m F+ttds18fu7i7nuzmphdN0eHpq7RXuXSXzfT4r5HLQ/PPuYP8fqLGJVYijMSDbWYi4oTAf01 snQkAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrLIsWRmVeSWpSXmKPExsVy+t/xy7oLdolGGrz8pmnRe+4kk8W6r7eZ LJo63rJazNr3kNXiytf3bBb3/nxgtbi8aw6bxfd/C5kcODwu9/UyeWz/9oDVY97JQI/73ceZ PLbs/8zo8XmTXABblJtNRmpiSmqRQmpecn5KZl66rVJoiJuuhZJCXmJuqq1ShK5vSJCSQlli TimQZ2SABhycA9yDlfTtEtwy/t2exlrwWLbiyPW5LA2MkyW6GDk5JARMJHb+eMcCYYtJXLi3 nq2LkYtDSGAJo8TD623sIAleAUGJH5PvARVxcDAL6Encv6gFEhYSeMYosfxrBYgtLOAs0fFj NjNIr4jAdSaJZ+s/M0IMOsQo8enadbBBbAKaEn8332SDGGonMWnpDbA4i4CqxP3F68HiogIR Eg87d4HFOQUcJa7vbmIEsZkF5CUOXnnOMoGRfxaSm2Yh3DQLSdUCRuZVjCKppcW56bnFRnrF ibnFpXnpesn5uZsYgVG17djPLTsYu94FH2IU4GBU4uGtWC8aKcSaWFZcmQt0Lwezkggv206g EG9KYmVValF+fFFpTmrxIUZToFsnMkuJJucDIz6vJN7QxNDc0tDI2MLC3MhISZx36ocr4UIC 6YklqdmpqQWpRTB9TBycUg2MVyckPjyUdObj/y166aKWV3u7YvIfHd+ifKo2bclRGft55uWH VG88nuKg0Sc8aa/Pz7fslbq+d4J/cVay96k0NtbIdP8UFJL7cUJ+smny3/l++9Lny3xeIHP5 6oWwdr1XoRLO3UzyXyd/47DjCX1kvJlPqIPP7dXX6un8au/muG6TXhDU/CtTiaU4I9FQi7mo OBEAW8TXFMACAAA= X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170512133704eucas1p173c76039bb4496322a287eac2b4d64d5 X-Msg-Generator: CA X-Sender-IP: 182.198.249.180 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: 20170512073245epcas4p28d5fa576d607281c056d99370785f70b X-RootMTR: 20170512073245epcas4p28d5fa576d607281c056d99370785f70b References: <20170512073232.yxknr556dal6isoh@phenom.ffwll.local> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12.05.2017 09:32, Daniel Vetter wrote: > On Thu, May 11, 2017 at 10:05:56AM +0100, Jose Abreu wrote: >> This series is a follow up from the discussion at [1]. We start by >> introducing crtc->mode_valid(), encoder->mode_valid() and >> bridge->mode_valid() callbacks which will be used in followup >> patches and also by cleaning the documentation a little bit. >> >> We proceed by introducing new helpers to call this new callbacks >> at 2/6. >> >> At 3/6 a helper function is introduced that calls all mode_valid() >> from a set of bridges. >> >> Next, at 4/6 we modify the connector probe helper so that only modes >> which are supported by a given bridge+encoder+crtc combination are >> probbed. >> >> At 5/6 we call all the mode_valid() callbacks for a given pipeline, >> except the connector->mode_valid one, so that the mode is validated. >> This is done before calling mode_fixup(). >> >> Finally, at 6/6 we use the new crtc->mode_valid() callback in arcpgu >> and remove the atomic_check() callback. >> >> [1] https://patchwork.kernel.org/patch/9702233/ >> >> Jose Abreu (6): >> drm: Add crtc/encoder/bridge->mode_valid() callbacks >> drm: Add drm_{crtc/encoder/connector}_mode_valid() >> drm: Introduce drm_bridge_mode_valid() >> drm: Use new mode_valid() helpers in connector probe helper >> drm: Use mode_valid() in atomic modeset >> drm: arc: Use crtc->mode_valid() callback >> >> Cc: Carlos Palminha >> Cc: Alexey Brodkin >> Cc: Ville Syrjälä >> Cc: Daniel Vetter >> Cc: Dave Airlie >> Cc: Andrzej Hajda >> Cc: Archit Taneja > Commented with an entire patch on patch 1, patches 2-5 are all > > Reviewed-by: Daniel Vetter > > But I think some more acks/r-bs would be really good, since this is quite > a bit change in the helper infrastructure. But otherwise ready for merging > imo. Can you pls also review my proposal for patch 1? > > Thanks, Daniel As the patchset improves many things, I would like to point here that there are still issues with mode probing at least in case of panels. Panels in general does not provide discrete list of supported modes, but they provide list of supported mode ranges (named display_timings), at the moment this is only addressed in drm_panel_funcs::get_timings, but drm core does not use it at all. Currently most of the panel drivers advertises only fixed list of arbitrary chosen modes, and if bridge/connector/encoder/crtc does not support it pipeline does not work, even if slightly different configuration could work. Of course there are workarounds but maybe it would be good to replace drm_connector::modes with drm_connector::timings. This way set of valid timings of the whole pipeline will be intersection of sets of valid timings of every component of the pipeline - quite straightforward and simple construct. As this is just an idea which came to me during patchset review, it is not backed by any code, but maybe it can be interesting for quick brainstorm. Regards Andrzej > >> drivers/gpu/drm/arc/arcpgu_crtc.c | 39 ++++++---- >> drivers/gpu/drm/drm_atomic_helper.c | 76 +++++++++++++++++++- >> drivers/gpu/drm/drm_bridge.c | 33 +++++++++ >> drivers/gpu/drm/drm_crtc_helper_internal.h | 13 ++++ >> drivers/gpu/drm/drm_probe_helper.c | 103 ++++++++++++++++++++++++++- >> include/drm/drm_bridge.h | 22 ++++++ >> include/drm/drm_modeset_helper_vtables.h | 110 ++++++++++++++++++++++------- >> 7 files changed, 348 insertions(+), 48 deletions(-) >> >> -- >> 1.9.1 >> >>