From patchwork Tue Nov 15 13:48:04 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jose Abreu X-Patchwork-Id: 9430773 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 B3A2B60469 for ; Wed, 16 Nov 2016 01:31:13 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A356C28C95 for ; Wed, 16 Nov 2016 01:31:13 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9804628D1C; Wed, 16 Nov 2016 01:31: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 0DEDA28C95 for ; Wed, 16 Nov 2016 01:31:13 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B05126E630; Wed, 16 Nov 2016 01:31:06 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from smtprelay.synopsys.com (smtprelay.synopsys.com [198.182.47.9]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4D3936E195 for ; Tue, 15 Nov 2016 13:48:22 +0000 (UTC) Received: from mailhost.synopsys.com (mailhost2.synopsys.com [10.13.184.66]) by smtprelay.synopsys.com (Postfix) with ESMTP id A2F1524E1054; Tue, 15 Nov 2016 05:48:21 -0800 (PST) Received: from mailhost.synopsys.com (localhost [127.0.0.1]) by mailhost.synopsys.com (Postfix) with ESMTP id 479857AE; Tue, 15 Nov 2016 05:48:21 -0800 (PST) Received: from us01wehtc1.internal.synopsys.com (us01wehtc1.internal.synopsys.com [10.12.239.235]) by mailhost.synopsys.com (Postfix) with ESMTP id 1D7C17AD; Tue, 15 Nov 2016 05:48:21 -0800 (PST) Received: from DE02WEHTCA.internal.synopsys.com (10.225.19.92) by us01wehtc1.internal.synopsys.com (10.12.239.231) with Microsoft SMTP Server (TLS) id 14.3.266.1; Tue, 15 Nov 2016 05:48:20 -0800 Received: from DE02WEHTCB.internal.synopsys.com (10.225.19.94) by DE02WEHTCA.internal.synopsys.com (10.225.19.92) with Microsoft SMTP Server (TLS) id 14.3.266.1; Tue, 15 Nov 2016 14:48:18 +0100 Received: from [10.0.2.15] (10.107.19.115) by DE02WEHTCB.internal.synopsys.com (10.225.19.80) with Microsoft SMTP Server (TLS) id 14.3.266.1; Tue, 15 Nov 2016 14:48:18 +0100 Subject: Re: [PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer" To: Daniel Vetter , "Sharma, Shashank" References: <20161114154937.GY31595@intel.com> <20161114162041.GZ31595@intel.com> <20161114164518.GA31595@intel.com> <20161115085135.ejys6afkqqz5ojyk@phenom.ffwll.local> <14e3f177-cd11-cf98-4f2d-0b2b1b4906a2@intel.com> <20161115100003.2pguapxruqwzocwr@phenom.ffwll.local> <218d2b25-1f7b-2ae4-30cc-0b05b2a0107d@intel.com> <20161115105246.gj6hwybcdurjgdgr@phenom.ffwll.local> From: Jose Abreu Message-ID: <6242286e-c2f4-d42b-28a8-5a45227866e2@synopsys.com> Date: Tue, 15 Nov 2016 13:48:04 +0000 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: <20161115105246.gj6hwybcdurjgdgr@phenom.ffwll.local> X-Originating-IP: [10.107.19.115] X-Mailman-Approved-At: Wed, 16 Nov 2016 01:30:52 +0000 Cc: Jose Abreu , "Jia, Lin A" , Akashdeep Sharma , Emil Velikov , dri-devel@lists.freedesktop.org, Daniel Vetter , Jim Bride 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 Hi, On 15-11-2016 10:52, Daniel Vetter wrote: > On Tue, Nov 15, 2016 at 03:36:02PM +0530, Sharma, Shashank wrote: >> On 11/15/2016 3:30 PM, Daniel Vetter wrote: >>> On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote: >>>> On 11/15/2016 2:21 PM, Daniel Vetter wrote: >>>>> On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote: >>>>>> In any case, I guess addition of a cap for aspect ratio should fix the >>>>>> current objections for this implementation. >>>>>> >>>>>> And I will keep it 0 by default, so that no aspect ratio information is >>>>>> added until userspace sets the cap to 1 on its own. >>>>> Note that cap = needs new userspace. >>>>> -Daniel >>>> I guess you mean a new libdrm, so yes, I will add this new cap in libdrm. >>>> Is that what you mean ? >>> Full stack solution, including enabling in an Xorg driver (or somewhere >>> else, we also have drm_hwcomposer as an option). >>> >>> And because that's probably going to take forever I'm leaning towards >>> revert again. Ville? >> Not really, with a kernel cap implementation, the code will be as it was >> before this patch series ( as good as revert ) >> And then when we want to enable it, we can add the corresponding Xserver >> patch. >> >> I guess the current problem is if is breaks something in some userspace, but >> if I am loading the flags only when the cap is set, we should be good. > This is not how new uabi gets merged, see: > > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements > > Userspace must be ready, or it will not land. The entire point of my > suggestion to just extend the display modes was to avoid the need for > userspace, but since existing userspace tries to be too clever already, > that didn't work out. > Thanks, Daniel @Ville I gave my tested by to these patches because I was facing the same scenario as Shashank: HDMI compliance. I believe we need to find a better way to handle this instead of just reverting. The HDMI spec is evolving so we also need to evolve. Of course that if this is breaking user space then we can revert and wait until we figure the best way to implement this. Anyway, this is a wild guess but I think the reason you are loosing modes may be because of drm_mode_equal_no_clocks_no_stereo() which is always expecting a aspect ratio flag to be defined. If there isn't one defined then it will unconditionally return false, even if the modes are equal. Can you please test this patch and see if it fixes on your side? WARNING: Not compile tested Best regards, Jose Miguel Abreu * drm_mode_equal_no_clocks_no_stereo - test modes for equality * @mode1: first mode @@ -995,7 +1016,7 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, mode1->vsync_end == mode2->vsync_end && mode1->vtotal == mode2->vtotal && mode1->vscan == mode2->vscan && - mode1->picture_aspect_ratio == mode2->picture_aspect_ratio && + drm_mode_equal_par(mode1, mode2) && (mode1->flags & ~DRM_MODE_FLAG_3D_MASK) == (mode2->flags & ~DRM_MODE_FLAG_3D_MASK)) return true; diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index ce6eeda..a7973a9 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -971,6 +971,27 @@ bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct } EXPORT_SYMBOL(drm_mode_equal_no_clocks); +static bool drm_mode_par_valid(const struct drm_display_mode *mode) +{ + switch (mode->picture_aspect_ratio) { + case HDMI_PICTURE_ASPECT_4_3: + case HDMI_PICTURE_ASPECT_16_9: + case HDMI_PICTURE_ASPECT_64_27: + case HDMI_PICTURE_ASPECT_256_135: + return true; + default: + return false; + } +} + +static bool drm_mode_equal_par(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) +{ + if (!drm_mode_par_valid(mode1) || !drm_mode_par_valid(mode2)) + return true; + return mode1->picture_aspect_ratio == mode2->picture_aspect_ratio; +} + /**