From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sam Ravnborg Subject: Re: [PATCH] drm: Split out drm_probe_helper.h Date: Mon, 21 Jan 2019 23:13:29 +0100 Message-ID: <20190121221329.GA6512@ravnborg.org> References: <20190116163442.12622-1-daniel.vetter@ffwll.ch> <20190116181018.GA27364@ravnborg.org> <20190117164541.GE3271@phenom.ffwll.local> <20190117174531.GA14041@ravnborg.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20190117174531.GA14041@ravnborg.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: Neil Armstrong , Daniel Vetter , Liviu Dudau , DRI Development , virtualization@lists.linux-foundation.org, Laurent Pinchart , Daniel Vetter , linux-stm32@st-md-mailman.stormreply.com, linux-samsung-soc@vger.kernel.org, Oleksandr Andrushchenko , amd-gfx@lists.freedesktop.org, linux-rockchip@lists.infradead.org, nouveau@lists.freedesktop.org, spice-devel@lists.freedesktop.org, Jani Nikula , linux-arm-msm@vger.kernel.org, intel-gfx@lists.freedesktop.org, etnaviv@lists.freedesktop.org, linux-mediatek@lists.infradead.org, Rodrigo Vivi , linux-tegra@vger.kernel.org, linux-amlogic@lists.infradead.org, linux-arm-kernel@lists.infradead.or List-Id: linux-tegra@vger.kernel.org SGkgRGFuaWVsIGV0IGFsLgoKPiA+IAo+ID4gWWVhaCB0aGUgZHJtX2NydGNfaGVscGVyLmggaGVh ZGVyIGlzIGEgYml0IHRoZSBtaW5pYXR1cmUgZHJtUC5oIGZvciBsZWdhY3kKPiA+IGttcyBkcml2 ZXJzLiBKdXN0IHJlbW92aW5nIGl0IGZyb20gYWxsIHRoZSBhdG9taWMgZHJpdmVycyBjYXVzZWQg bG90cyBvZgo+ID4gZmFsbG91dCwgSSBleHBlY3QgZXZlbiBtb3JlIGlmIHlvdSBlbnRpcmVseSBy ZW1vdmUgdGhlIGluY2x1ZGVzIGl0IGhhcy4KPiA+IE1heWJlIGEgdG9kbywgY2FyZSB0byBwbHMg Y3JlYXRlIHRoYXQgcGF0Y2ggc2luY2UgaXQncyB5b3VyIGlkZWE/Cj4gCj4gVGhlIG1haW4gcmVh c29uIEkgYmFpbGVkIG91dCBpbml0aWFsbHkgd2FzIHRoYXQgdGhpcyB3b3VsZCBjcmVhdGUKPiBz bWFsbCBjaGFuZ2VzIHRvIHNldmVyYWwgb3RoZXJ3aXNlIHNlbGRvbWx5IHRvdWNoZWQgZmlsZXMu Cj4gQW5kIHRoZW4gd2Ugd291bGQgbGF0ZXIgY29tZSBhbmQgcmVtb3ZlIGRybVAuaCAtIHNvIGxv dHMgb2YKPiBzbWFsbCBidXQgaW5jcmVtZW50YWwgY2hhbmdlcyB0byB0aGUgc2FtZSBvdGhlcndp c2Ugc2VsZG9tbHkKPiBlZGl0ZWQgZmlsZXMuCj4gQW5kIHRoZSBqb2Igd2FzIG9ubHkgcGFydGlh bGx5IGRvbmUuCj4gCj4gSSB3aWxsIHRyeSB0byBleHBlcmltZW50IHdpdGggYW4gYXBwcm9hY2gg d2hlcmUgSSBjbGVhbiB1cCB0aGUKPiBpbmNsdWRlL2RybS8qLmggZmlsZXMgYSBsaXR0bGUgKGxp a2Ugc3VnZ2VzdGVkIGFib3ZlLCArZGVsZXRlIGRybVAuaAo+IGFuZCBtYXliZSBhIGJpdCBtb3Jl KS4KPiAKPiBUaGVuIHRvIHRyeSBvbiBhIGRyaXZlciBieSBkcml2ZXIgYmFzaXMgdG8gbWFrZSBp dCBidWlsZCB3aXRoIGEKPiBjbGVhbmVkIHNldCBvZiBpbmNsdWRlIGZpbGVzLgo+IEkgaG9wZSB0 aGF0IHRoZSBjbGVhbmVkIHVwIGRyaXZlciBjYW4gc3RpbGwgYnVpbGQgd2l0aG91dCB0aGUKPiBj bGVhbmVkIGhlYWRlciBmaWxlcyBzbyB0aGUgY2hhbmdlcyBjYW4gYmUgc3VibWl0dGVkIHBpZWNl bWFsLgo+IAo+IFdpbGwgZG8gc28gd2l0aCBhbiBleWUgb24gdGhlIGxlc3NlciBtYWludGFpbmVk IGRyaXZlcnMgdG8gdHJ5IGl0Cj4gb3V0IHRvIGF2b2lkIGNyZWF0aW5nIHRvbyBtdWNoIGNocnVu Y2ggZm9yIG90aGVycy4KCkkgaGF2ZSBub3cgYSBmZXcgcGF0Y2hlcyBxdWV1ZWQsIGJ1dCB0aGUg cmVzdWx0IGlzIG5vdCB0b28gcHJldHR5LgpJIGRpZCB0aGUgZm9sbG93aW5nOgoKLSBGb3IgYWxs IGZpbGVzIGluIGluY2x1ZGUvZHJtLyouaCB0aGUgc2V0IG9mIGluY2x1ZGUgZmlsZXMKICB3ZXJl IGFkanVzdGVkIHRvIHRoZSBtaW5pbXVtIG51bWJlciBvZiBmaWxlcyByZXF1aXJlZCB0byBtYWtl CiAgdGhlbSBidWlsZCB3aXRob3V0IGFueSBvdGhlciBmaWxlcyBpbmNsdWRlZCBmaXJzdC4KCiAg Q3JlYXRlZCBvbmUgLmMgZmlsZSBmb3IgZWFjaCAuaCBmaWxlLiBUaGVuIGluY2x1ZGVkIHRoZSAu aAogIGZpbGUgYW5kIGFkanVzdGVkIHRvIHRoZSBtaW5pbWFsIHNldCBvZiBpbmNsdWRlIGZpbGVz LgogIEluIHRoZSBwcm9jZXNzIGEgbG90IG9mIGZvcndhcmRzIHdlcmUgYWRkZWQuCgotIERlbGV0 ZWQgZHJtUC5oCgotIEZpeGVkIGJ1aWxkIG9mIGEgZmV3IGRyaXZlcnM6IHN0aSwgdGlsY2RjLCBn bWE1MDAsIHR2ZTIwMCwgdmlhCgpTb21lIG9ic2VydmF0aW9uczoKCi0gS2lsbGluZyBhbGwgdGhl IGluY2x1ZGVzIG5vdCBuZWVkZWQgaW4gdGhlIGhlYWRlcnMgZmlsZXMKICByZXN1bHRzIGluIGEg YSBsb3Qgb2YgZXh0cmEgY2hhbmdlcy4KICBFeGFtcGxlczoKICAgIGRybV9tb2RzZXNldF9oZWxw ZXJfdnRhYmxlcy5oIGlzIG5vIGxvbmdlcgogICAgaW5jbHVkZWQgYnkgYW55b25lLCBzbyBuZWVk cyB0byBiZSBhZGRlZCBpbiBtYW55IGZpbGVzCgogICAgZHJtX2F0b21pY19zdGF0ZV9oZWxwZXIu aCBpcyBubyBsb25nZXIgaW5jbHVkZWQKICAgIGJ5IGFueW9uZSBzbyBsaWtld2lzZSBuZWVkcyB0 byBiZSBhZGRlZCBpbiBtYW55IGZpbGVzCgotIEl0IGlzIHZlcnkgdGVkaW91cyB0byBkbyB0aGlz IHByb3Blcmx5LgogIFRoZSBwcm9jZXNzIEkgZm9sbG93ZWQgd2FzOgogIC0gZGVsZXRlIC8gY29t bWVudCBvdXQgYWxsIGluY2x1ZGUgZmlsZXMKICAtIGFkZCBiYWNrIHRoZSBvYnZpb3VzIGZyb20g YSBxdWljayBzY2FuIG9mIHRoZSBjb2RlCiAgLSBidWlsZCAtIGZpeCAtIGJ1aWxkIC0gZml4IC0g YnVpbGQgLSBmaXggLi4uCiAgLSAgIG5leHQgZmlsZS4uLgoKLSBUaGUgcmVzdWx0IGlzIGVycm9y cHJvbmUgYXMgb25seSB0aGUgYWxseWVzY29uZmlnICsgYWxsbW9kY29uZmlnCiAgdmFyaWFudHMg YXJlIHRlc3RlZC4gQnV0IHJlYWxsaWZlIGNvbmZpZ3VyYXRpb25zIGFyZSBtb3JlIGRpdmVyc2Uu CgpDdXJyZW50IGRpZmZzdGF0OgogICAxMTEgZmlsZXMgY2hhbmdlZCwgNzcxIGluc2VydGlvbnMo KyksIDQwMSBkZWxldGlvbnMoLSkKClRoaXMgaXMgZm9yIHRoZSA1IGRyaXZlcnMgYWxvbmUgYW5k IG5vdCB0aGUgaGVhZGVyIGNsZWFudXAuClNvIGxvbmcgc3Rvcnkgc2hvcnQgLSB0aGlzIGlzIG5v dCBnb29kIGFuZCBub3QgdGhlIHdheSBmb3J3YXJkLgoKSSB3aWxsIHRyeSB0byBjb21lIHVwIHdp dGggYSBmZXcgaW1wcm92ZW1lbnRzIHRvIG1ha2UgdGhlCmhlYWRlcnMgZmlsZXMgc2VsZmNvbnRh aW5lZCwgYnV0IHJlc3RyaWN0ZWQgdG8gdGhlIGNoYW5nZXMgdGhhdAphZGQgZm9yd2FyZHMvaW5j bHVkZSB0byBhdm9pZCB0aGUgY2hydW5jaCBpbiBhbGwgdGhlIGRyaXZlcnMuCgpBbmQgdGhlbiBw b3N0IGZvciByZXZpZXcgYSBmZXcgcGF0Y2hlcyB0byBjbGVhbiB1cCBzb21lIGhlYWRlcnMuCklm IHRoZSBjbGVhbnVwIGdldHMgYSBnbyBJIHdpbGwgdHJ5IHRvIHBlcnN1YWRlIHRoZSBpbnRyb2R1 Y3Rpb24Kb2YgdGhlc2UuClRoaXMgd2lsbCBpbmNsdWRlLCBidXQgd2lsbCBub3QgYmUgbGltaXRl ZCB0bywgdGhlIGFib3ZlIG1lbnRpb25lZApkcm1fY3J0Y19oZWxwZXIuaCBoZWFkZXIgZmlsZS4K CkZvciBub3cgdG9vIG11Y2ggdGltZSB3YXMgYWxyZWFkeSBzcGVudCBvbiB0aGlzLCBzbyBpdCBp cyBhdCB0aGUKbW9tZW50IHB1c2hlZCBiYWNrIG9uIG15IFRPRE8gbGlzdC4KVGhpcyBtYWlsIHNl cnZlIGFsc28gYXMgYSBraW5kIG9mICJ3aGVyZSBoYWQgSSBsZWZ0Iiwgd2hlbi9pZiBJCnBpY2sg dGhpcyB1cCBhZ2Fpbi4KCklmIHRoZXJlIGFyZSBhbnlvbmUgdGhhdCBrbm93cyBzb21lIHRvb2xp bmcgdGhhdCBjYW4gaGVscCBpbiB0aGUKcHJvY2VzcyBvZiBhZGp1c3RpbmcgdGhlIGhlYWRlciBm aWxlcyBJIGFtIGFsbCBlYXJzLgoKCVNhbQpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5m cmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0 aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E2F4AC3713C for ; Mon, 21 Jan 2019 22:13:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id ACD57217D6 for ; Mon, 21 Jan 2019 22:13:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727605AbfAUWNk (ORCPT ); Mon, 21 Jan 2019 17:13:40 -0500 Received: from asavdk4.altibox.net ([109.247.116.15]:52573 "EHLO asavdk4.altibox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726244AbfAUWNj (ORCPT ); Mon, 21 Jan 2019 17:13:39 -0500 Received: from ravnborg.org (unknown [158.248.194.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by asavdk4.altibox.net (Postfix) with ESMTPS id 41C438032F; Mon, 21 Jan 2019 23:13:32 +0100 (CET) Date: Mon, 21 Jan 2019 23:13:29 +0100 From: Sam Ravnborg To: Daniel Vetter Cc: Neil Armstrong , Daniel Vetter , Liviu Dudau , DRI Development , virtualization@lists.linux-foundation.org, Laurent Pinchart , Daniel Vetter , linux-stm32@st-md-mailman.stormreply.com, linux-samsung-soc@vger.kernel.org, Oleksandr Andrushchenko , amd-gfx@lists.freedesktop.org, linux-rockchip@lists.infradead.org, nouveau@lists.freedesktop.org, spice-devel@lists.freedesktop.org, Jani Nikula , linux-arm-msm@vger.kernel.org, intel-gfx@lists.freedesktop.org, etnaviv@lists.freedesktop.org, linux-mediatek@lists.infradead.org, Rodrigo Vivi , linux-tegra@vger.kernel.org, linux-amlogic@lists.infradead.org, linux-arm-kernel@lists.infradead.org, xen-devel@lists.xen.org, linux-renesas-soc@vger.kernel.org, Alex Deucher , freedreno@lists.freedesktop.org Subject: Re: [PATCH] drm: Split out drm_probe_helper.h Message-ID: <20190121221329.GA6512@ravnborg.org> References: <20190116163442.12622-1-daniel.vetter@ffwll.ch> <20190116181018.GA27364@ravnborg.org> <20190117164541.GE3271@phenom.ffwll.local> <20190117174531.GA14041@ravnborg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190117174531.GA14041@ravnborg.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-CMAE-Score: 0 X-CMAE-Analysis: v=2.3 cv=UpRNyd4B c=1 sm=1 tr=0 a=UWs3HLbX/2nnQ3s7vZ42gw==:117 a=UWs3HLbX/2nnQ3s7vZ42gw==:17 a=kj9zAlcOel0A:10 a=HoD5qxnl0p1esR3YI2YA:9 a=CjuIK1q_8ugA:10 Sender: linux-renesas-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-renesas-soc@vger.kernel.org Hi Daniel et al. > > > > Yeah the drm_crtc_helper.h header is a bit the miniature drmP.h for legacy > > kms drivers. Just removing it from all the atomic drivers caused lots of > > fallout, I expect even more if you entirely remove the includes it has. > > Maybe a todo, care to pls create that patch since it's your idea? > > The main reason I bailed out initially was that this would create > small changes to several otherwise seldomly touched files. > And then we would later come and remove drmP.h - so lots of > small but incremental changes to the same otherwise seldomly > edited files. > And the job was only partially done. > > I will try to experiment with an approach where I clean up the > include/drm/*.h files a little (like suggested above, +delete drmP.h > and maybe a bit more). > > Then to try on a driver by driver basis to make it build with a > cleaned set of include files. > I hope that the cleaned up driver can still build without the > cleaned header files so the changes can be submitted piecemal. > > Will do so with an eye on the lesser maintained drivers to try it > out to avoid creating too much chrunch for others. I have now a few patches queued, but the result is not too pretty. I did the following: - For all files in include/drm/*.h the set of include files were adjusted to the minimum number of files required to make them build without any other files included first. Created one .c file for each .h file. Then included the .h file and adjusted to the minimal set of include files. In the process a lot of forwards were added. - Deleted drmP.h - Fixed build of a few drivers: sti, tilcdc, gma500, tve200, via Some observations: - Killing all the includes not needed in the headers files results in a a lot of extra changes. Examples: drm_modseset_helper_vtables.h is no longer included by anyone, so needs to be added in many files drm_atomic_state_helper.h is no longer included by anyone so likewise needs to be added in many files - It is very tedious to do this properly. The process I followed was: - delete / comment out all include files - add back the obvious from a quick scan of the code - build - fix - build - fix - build - fix ... - next file... - The result is errorprone as only the allyesconfig + allmodconfig variants are tested. But reallife configurations are more diverse. Current diffstat: 111 files changed, 771 insertions(+), 401 deletions(-) This is for the 5 drivers alone and not the header cleanup. So long story short - this is not good and not the way forward. I will try to come up with a few improvements to make the headers files selfcontained, but restricted to the changes that add forwards/include to avoid the chrunch in all the drivers. And then post for review a few patches to clean up some headers. If the cleanup gets a go I will try to persuade the introduction of these. This will include, but will not be limited to, the above mentioned drm_crtc_helper.h header file. For now too much time was already spent on this, so it is at the moment pushed back on my TODO list. This mail serve also as a kind of "where had I left", when/if I pick this up again. If there are anyone that knows some tooling that can help in the process of adjusting the header files I am all ears. Sam From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, USER_AGENT_MUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3036CC3712F for ; Mon, 21 Jan 2019 22:13:45 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F2AE920861 for ; Mon, 21 Jan 2019 22:13:44 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="tpOjC+i4" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F2AE920861 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ravnborg.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=mq+SzUQ7awDo5Vlykz5zkK0RZAD3mpMQWKvnmLDJAu0=; b=tpOjC+i4jgIiMd G28PPyjkS8uslmzC1dSjuapfZa81M8wCSkVLzU3ZgejCVydcEmJs2jLyUNmXOOwxwQMCq5qgML8GA wvfzmHe0hetphqiwOTg3fW8bvYEWDKiXfuUA23j61V6Z4H8CRawwNDBpxu9VfP56qHdT179PG7XMb gaLgosfHH2arcNoWFbKYWIjcU3kPb/chKDXLf6o9w2E3d2T4pWOWPp5o0E54bCxwm+UBixap/G22O sBq85cZl2rTnwt59JA1g3VqvMGR2fQvpg+YVOtbBf3Crbh39n/o17q/zx37TEruIJe3yZH3wlUGKt gLKTZdr0mGhH6J+7iBKA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1glhp9-0000kS-Th; Mon, 21 Jan 2019 22:13:43 +0000 Received: from asavdk4.altibox.net ([109.247.116.15]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1glhp5-0000jZ-2u; Mon, 21 Jan 2019 22:13:41 +0000 Received: from ravnborg.org (unknown [158.248.194.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by asavdk4.altibox.net (Postfix) with ESMTPS id 41C438032F; Mon, 21 Jan 2019 23:13:32 +0100 (CET) Date: Mon, 21 Jan 2019 23:13:29 +0100 From: Sam Ravnborg To: Daniel Vetter Subject: Re: [PATCH] drm: Split out drm_probe_helper.h Message-ID: <20190121221329.GA6512@ravnborg.org> References: <20190116163442.12622-1-daniel.vetter@ffwll.ch> <20190116181018.GA27364@ravnborg.org> <20190117164541.GE3271@phenom.ffwll.local> <20190117174531.GA14041@ravnborg.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190117174531.GA14041@ravnborg.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-CMAE-Score: 0 X-CMAE-Analysis: v=2.3 cv=UpRNyd4B c=1 sm=1 tr=0 a=UWs3HLbX/2nnQ3s7vZ42gw==:117 a=UWs3HLbX/2nnQ3s7vZ42gw==:17 a=kj9zAlcOel0A:10 a=HoD5qxnl0p1esR3YI2YA:9 a=CjuIK1q_8ugA:10 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190121_141339_480126_BC961E3D X-CRM114-Status: GOOD ( 25.13 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Neil Armstrong , Daniel Vetter , Liviu Dudau , DRI Development , virtualization@lists.linux-foundation.org, Laurent Pinchart , Daniel Vetter , linux-stm32@st-md-mailman.stormreply.com, linux-samsung-soc@vger.kernel.org, Oleksandr Andrushchenko , amd-gfx@lists.freedesktop.org, linux-rockchip@lists.infradead.org, nouveau@lists.freedesktop.org, spice-devel@lists.freedesktop.org, Jani Nikula , linux-arm-msm@vger.kernel.org, intel-gfx@lists.freedesktop.org, etnaviv@lists.freedesktop.org, linux-mediatek@lists.infradead.org, Rodrigo Vivi , linux-tegra@vger.kernel.org, linux-amlogic@lists.infradead.org, linux-arm-kernel@lists.infradead.org, xen-devel@lists.xen.org, linux-renesas-soc@vger.kernel.org, Alex Deucher , freedreno@lists.freedesktop.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Daniel et al. > > > > Yeah the drm_crtc_helper.h header is a bit the miniature drmP.h for legacy > > kms drivers. Just removing it from all the atomic drivers caused lots of > > fallout, I expect even more if you entirely remove the includes it has. > > Maybe a todo, care to pls create that patch since it's your idea? > > The main reason I bailed out initially was that this would create > small changes to several otherwise seldomly touched files. > And then we would later come and remove drmP.h - so lots of > small but incremental changes to the same otherwise seldomly > edited files. > And the job was only partially done. > > I will try to experiment with an approach where I clean up the > include/drm/*.h files a little (like suggested above, +delete drmP.h > and maybe a bit more). > > Then to try on a driver by driver basis to make it build with a > cleaned set of include files. > I hope that the cleaned up driver can still build without the > cleaned header files so the changes can be submitted piecemal. > > Will do so with an eye on the lesser maintained drivers to try it > out to avoid creating too much chrunch for others. I have now a few patches queued, but the result is not too pretty. I did the following: - For all files in include/drm/*.h the set of include files were adjusted to the minimum number of files required to make them build without any other files included first. Created one .c file for each .h file. Then included the .h file and adjusted to the minimal set of include files. In the process a lot of forwards were added. - Deleted drmP.h - Fixed build of a few drivers: sti, tilcdc, gma500, tve200, via Some observations: - Killing all the includes not needed in the headers files results in a a lot of extra changes. Examples: drm_modseset_helper_vtables.h is no longer included by anyone, so needs to be added in many files drm_atomic_state_helper.h is no longer included by anyone so likewise needs to be added in many files - It is very tedious to do this properly. The process I followed was: - delete / comment out all include files - add back the obvious from a quick scan of the code - build - fix - build - fix - build - fix ... - next file... - The result is errorprone as only the allyesconfig + allmodconfig variants are tested. But reallife configurations are more diverse. Current diffstat: 111 files changed, 771 insertions(+), 401 deletions(-) This is for the 5 drivers alone and not the header cleanup. So long story short - this is not good and not the way forward. I will try to come up with a few improvements to make the headers files selfcontained, but restricted to the changes that add forwards/include to avoid the chrunch in all the drivers. And then post for review a few patches to clean up some headers. If the cleanup gets a go I will try to persuade the introduction of these. This will include, but will not be limited to, the above mentioned drm_crtc_helper.h header file. For now too much time was already spent on this, so it is at the moment pushed back on my TODO list. This mail serve also as a kind of "where had I left", when/if I pick this up again. If there are anyone that knows some tooling that can help in the process of adjusting the header files I am all ears. Sam _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, USER_AGENT_MUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC81CC3712F for ; Mon, 21 Jan 2019 22:13:51 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 8482720861 for ; Mon, 21 Jan 2019 22:13:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="ddAlpP/0" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8482720861 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ravnborg.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-amlogic-bounces+linux-amlogic=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=eCX812RAibTyHSOUsuum3Dez2kddHyzpFnhQ0hZgp+M=; b=ddAlpP/0CqqfZo ByDXg2k/dAOkWVn5kRVy1RPXqbhzM0BGyVI4fc8XJRNzRz7yYCCGvA1kUgeCHVrKrZUPXU9KVc9bn qs3/oh0f2E/YP90WhynBRSS65IkgkK/I86k2uUu6f4DS20ezd6mNAdfJhoWOeut4q+huXDGEHUmBY erLZZ0ILZKP+JnGkbJDIp3IBmWo1fjjeSAQI2KOBVVZlmmYCfOA5jjACW+vOXKrQHIWsdH0MqFO6d 1hUfdmitpasnQnHIBTgn+jqUonh19sUuzhs2Kc8Sr2BGGQKgvKpTGB4SKupeOxTAy7YqoYNH2Y0ef f+Xw59yxBnhQzc5v01zQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1glhp8-0000k4-Pt; Mon, 21 Jan 2019 22:13:42 +0000 Received: from asavdk4.altibox.net ([109.247.116.15]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1glhp5-0000jZ-2u; Mon, 21 Jan 2019 22:13:41 +0000 Received: from ravnborg.org (unknown [158.248.194.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by asavdk4.altibox.net (Postfix) with ESMTPS id 41C438032F; Mon, 21 Jan 2019 23:13:32 +0100 (CET) Date: Mon, 21 Jan 2019 23:13:29 +0100 From: Sam Ravnborg To: Daniel Vetter Subject: Re: [PATCH] drm: Split out drm_probe_helper.h Message-ID: <20190121221329.GA6512@ravnborg.org> References: <20190116163442.12622-1-daniel.vetter@ffwll.ch> <20190116181018.GA27364@ravnborg.org> <20190117164541.GE3271@phenom.ffwll.local> <20190117174531.GA14041@ravnborg.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190117174531.GA14041@ravnborg.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-CMAE-Score: 0 X-CMAE-Analysis: v=2.3 cv=UpRNyd4B c=1 sm=1 tr=0 a=UWs3HLbX/2nnQ3s7vZ42gw==:117 a=UWs3HLbX/2nnQ3s7vZ42gw==:17 a=kj9zAlcOel0A:10 a=HoD5qxnl0p1esR3YI2YA:9 a=CjuIK1q_8ugA:10 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190121_141339_480126_BC961E3D X-CRM114-Status: GOOD ( 25.13 ) X-BeenThere: linux-amlogic@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Neil Armstrong , Daniel Vetter , Liviu Dudau , DRI Development , virtualization@lists.linux-foundation.org, Laurent Pinchart , Daniel Vetter , linux-stm32@st-md-mailman.stormreply.com, linux-samsung-soc@vger.kernel.org, Oleksandr Andrushchenko , amd-gfx@lists.freedesktop.org, linux-rockchip@lists.infradead.org, nouveau@lists.freedesktop.org, spice-devel@lists.freedesktop.org, Jani Nikula , linux-arm-msm@vger.kernel.org, intel-gfx@lists.freedesktop.org, etnaviv@lists.freedesktop.org, linux-mediatek@lists.infradead.org, Rodrigo Vivi , linux-tegra@vger.kernel.org, linux-amlogic@lists.infradead.org, linux-arm-kernel@lists.infradead.org, xen-devel@lists.xen.org, linux-renesas-soc@vger.kernel.org, Alex Deucher , freedreno@lists.freedesktop.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-amlogic" Errors-To: linux-amlogic-bounces+linux-amlogic=archiver.kernel.org@lists.infradead.org Hi Daniel et al. > > > > Yeah the drm_crtc_helper.h header is a bit the miniature drmP.h for legacy > > kms drivers. Just removing it from all the atomic drivers caused lots of > > fallout, I expect even more if you entirely remove the includes it has. > > Maybe a todo, care to pls create that patch since it's your idea? > > The main reason I bailed out initially was that this would create > small changes to several otherwise seldomly touched files. > And then we would later come and remove drmP.h - so lots of > small but incremental changes to the same otherwise seldomly > edited files. > And the job was only partially done. > > I will try to experiment with an approach where I clean up the > include/drm/*.h files a little (like suggested above, +delete drmP.h > and maybe a bit more). > > Then to try on a driver by driver basis to make it build with a > cleaned set of include files. > I hope that the cleaned up driver can still build without the > cleaned header files so the changes can be submitted piecemal. > > Will do so with an eye on the lesser maintained drivers to try it > out to avoid creating too much chrunch for others. I have now a few patches queued, but the result is not too pretty. I did the following: - For all files in include/drm/*.h the set of include files were adjusted to the minimum number of files required to make them build without any other files included first. Created one .c file for each .h file. Then included the .h file and adjusted to the minimal set of include files. In the process a lot of forwards were added. - Deleted drmP.h - Fixed build of a few drivers: sti, tilcdc, gma500, tve200, via Some observations: - Killing all the includes not needed in the headers files results in a a lot of extra changes. Examples: drm_modseset_helper_vtables.h is no longer included by anyone, so needs to be added in many files drm_atomic_state_helper.h is no longer included by anyone so likewise needs to be added in many files - It is very tedious to do this properly. The process I followed was: - delete / comment out all include files - add back the obvious from a quick scan of the code - build - fix - build - fix - build - fix ... - next file... - The result is errorprone as only the allyesconfig + allmodconfig variants are tested. But reallife configurations are more diverse. Current diffstat: 111 files changed, 771 insertions(+), 401 deletions(-) This is for the 5 drivers alone and not the header cleanup. So long story short - this is not good and not the way forward. I will try to come up with a few improvements to make the headers files selfcontained, but restricted to the changes that add forwards/include to avoid the chrunch in all the drivers. And then post for review a few patches to clean up some headers. If the cleanup gets a go I will try to persuade the introduction of these. This will include, but will not be limited to, the above mentioned drm_crtc_helper.h header file. For now too much time was already spent on this, so it is at the moment pushed back on my TODO list. This mail serve also as a kind of "where had I left", when/if I pick this up again. If there are anyone that knows some tooling that can help in the process of adjusting the header files I am all ears. Sam _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic