Received: by 2002:a05:6a10:af89:0:0:0:0 with SMTP id iu9csp2690310pxb; Mon, 17 Jan 2022 03:56:34 -0800 (PST) X-Google-Smtp-Source: ABdhPJwpXhxFmneyc8fZ+WPLpWzMXYLTh+xEbJ93cBafgW70QZ+ppIXayb26BVtHe99JQavYPbT7 X-Received: by 2002:a63:d314:: with SMTP id b20mr18624458pgg.207.1642420594744; Mon, 17 Jan 2022 03:56:34 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1642420594; cv=none; d=google.com; s=arc-20160816; b=sfASh8qRJZBtHHcIRMbIx36PSB3tHjJeAFbCkyyAHKqGBnCVg9OHwKH3y35dST+Kzb aMNbQOJzjdvbAb5Z4wWMbc6Wqm6Eyw9EePjJIUMQNmflOjLlHc/NHnn28Og4gF/uo4TL kS2jWiIQ9A8hgZ365BIRZvq/2C2QO4ba43tY8e/XcPzQBgSHnvm054CaxW0gnIJK50Dx knQ1FKZr1s4xUK1e1bHLiwNhCvn/ijuDMo0C63BCxJ+sCVuOyhx0n5yw18vDZxuDy2nh XIupZgGzSazEpDN6JgPPLdXXycBaD1ODYe82/5JePMrQUHQrrYYGEKktKUqfOla8nMFg dG9Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:in-reply-to:from :references:to:content-language:subject:user-agent:mime-version:date :message-id:dkim-signature; bh=qUMGXQA24GuCkD0foGZ5EQ/jWXa2C4jCsPx92Oi0DR4=; b=J3IyRmnhlf0gNoqfPfVhHhKYijT3djOBWt/+1d6DcbxYDl+9VkCkU4ZE3AeZTowhVF eqQooXDWDyqPTQsuvMkLkpNnKhMloG9B9fo2rWg5gdgs/6LVmqJZ00jVq3Cv8NiyByrJ 0XG6uPcNslhOiQcbyZmPgcAXjFeCraml0bDI68Q5IV+QO7QCIUsNqKGxj+gk4L6AJYHN B+BjPW/UkPNnNGC0vmKjacdoK1DYEieNFUbMWtPQmHsuQHGcOYIxLMsuKUwNm1jlB+BX uoNniXXPd6qzfFApbhDlo/DB3D5dgRbNbGbc9KfM/28NnB1Z2PDcrrSjee1bBek95JIf zifw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@igel-co-jp.20210112.gappssmtp.com header.s=20210112 header.b=3JcalMkL; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id r190si14255498pgr.127.2022.01.17.03.56.08; Mon, 17 Jan 2022 03:56:34 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@igel-co-jp.20210112.gappssmtp.com header.s=20210112 header.b=3JcalMkL; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236866AbiAQCpi (ORCPT + 99 others); Sun, 16 Jan 2022 21:45:38 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47870 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232774AbiAQCpi (ORCPT ); Sun, 16 Jan 2022 21:45:38 -0500 Received: from mail-pg1-x52b.google.com (mail-pg1-x52b.google.com [IPv6:2607:f8b0:4864:20::52b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CAE46C061574 for ; Sun, 16 Jan 2022 18:45:37 -0800 (PST) Received: by mail-pg1-x52b.google.com with SMTP id f8so9565899pgf.8 for ; Sun, 16 Jan 2022 18:45:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=igel-co-jp.20210112.gappssmtp.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :references:from:in-reply-to:content-transfer-encoding; bh=qUMGXQA24GuCkD0foGZ5EQ/jWXa2C4jCsPx92Oi0DR4=; b=3JcalMkLwRqtx0dh+FBtyQAGr4zEik0NNokh7CZm9OjxATON/gXp5yImWlNGSIthv+ GKYZ+O5JUE82OShMSyjYbUfxwuerQa/dKw8d7y9kVZm3zCqZJOKr0d1o6ayrotoo3lAJ MmQZ1GZ5ae53FWqTbV1p4NViMmmFmTtzBh4sCCWV8S0DvMixlOTHfAfZkbOp699BGmZc afbQ9vjEADSw0zK+vnLf4MyjIuPu5pTDn2igIrZ7D0ZnOdZrUU1kZWmPomddoEpg5LNj 9nnyRMfVFRPxSyImBzr3BFn7mjlhalInCzqMRsNZTIpyJ2B3c6mTjAHrA4Tyv0wFWSRS SZ5Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=qUMGXQA24GuCkD0foGZ5EQ/jWXa2C4jCsPx92Oi0DR4=; b=Qq3mYCsv7XzHEbJeB19AvJOpP6BHVBZCqD+nNWYrSCg49CXJJ3do45CK3J4rfkQZnt p/mb0W81FUngH5DLhXhh2yAuULV0fIRb4K+y7rfwhdOVpkV/Ex1fjCurbk2gWxsIiFxK UVFM0zLFEGLibQdcaaRhmAcgubemQUD2F1sZk6yWUw+804O9n5gZMyYptDqTqaJnCufc OgA+6IVhIj75zvz1I9f7o+WcaI7LvAqfL0wZ0wIcJlVZm+wXeluK+LVPEIzSCG+r1LNT 1O+ml/uE5BcsHjaaCWPy6CH7OKKxx0AWkUa6l6+rNSUXm2NO4qKk5h1tUCAJTSE8s8hh fhqQ== X-Gm-Message-State: AOAM530I4nqpIMyasqGpxKJ7KeqyWRH6Lz4JkgUI/2zFzbcFfM3mx81V ZjbaCnGP/u+MLcf5DnGbereF7Q== X-Received: by 2002:a05:6a00:188a:b0:4c2:faa1:b6ed with SMTP id x10-20020a056a00188a00b004c2faa1b6edmr10839523pfh.54.1642387536771; Sun, 16 Jan 2022 18:45:36 -0800 (PST) Received: from [10.16.129.73] (napt.igel.co.jp. [219.106.231.132]) by smtp.gmail.com with ESMTPSA id c21sm2183587pgw.41.2022.01.16.18.45.29 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 16 Jan 2022 18:45:36 -0800 (PST) Message-ID: Date: Mon, 17 Jan 2022 11:45:27 +0900 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.4.1 Subject: Re: [RFC PATH 1/3] drm: add support modifiers for drivers whose planes only support linear layout Content-Language: en-US To: dri-devel@lists.freedesktop.org, Alex Deucher , =?UTF-8?Q?Christian_K=c3=b6nig?= , "Pan, Xinhui" , David Airlie , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Ben Skeggs , =?UTF-8?Q?Michel_D=c3=a4nzer?= , Simon Ser , Qingqing Zhuo , Bas Nieuwenhuizen , Mark Yacoub , Sean Paul , Evan Quan , Andy Shevchenko , Petr Mladek , Sakari Ailus , Lee Jones , Abhinav Kumar , Dmitry Baryshkov , Rob Clark , amd-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, nouveau@lists.freedesktop.org, Damian Hobson-Garcia , Takanari Hayama References: <20211222052727.19725-1-etom@igel.co.jp> <20211222052727.19725-2-etom@igel.co.jp> From: Esaki Tomohito In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Thank you for your reviews. On 2022/01/15 1:50, Daniel Vetter wrote: > On Wed, Dec 22, 2021 at 02:27:25PM +0900, Tomohito Esaki wrote: >> The LINEAR modifier is advertised as default if a driver doesn't specify >> modifiers. However, there are legacy drivers such as radeon that do not >> support modifiers but infer the actual layout of the underlying buffer. >> Therefore, a new flag not_support_fb_modifires is introduced for these >> legacy drivers. Allow_fb_modifiers will be replaced with this new flag. >> >> Signed-off-by: Tomohito Esaki >> --- >> drivers/gpu/drm/drm_plane.c | 34 ++++++++++++++++++++++++++-------- >> include/drm/drm_mode_config.h | 10 ++++++++++ >> include/drm/drm_plane.h | 3 +++ >> 3 files changed, 39 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c >> index 82afb854141b..75308ee240c0 100644 >> --- a/drivers/gpu/drm/drm_plane.c >> +++ b/drivers/gpu/drm/drm_plane.c >> @@ -161,6 +161,16 @@ modifiers_ptr(struct drm_format_modifier_blob *blob) >> return (struct drm_format_modifier *)(((char *)blob) + blob->modifiers_offset); >> } >> >> +static bool check_format_modifier(struct drm_plane *plane, uint32_t format, >> + uint64_t modifier) >> +{ >> + if (plane->funcs->format_mod_supported) >> + return plane->funcs->format_mod_supported(plane, format, >> + modifier); >> + >> + return modifier == DRM_FORMAT_MOD_LINEAR; >> +} >> + >> static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane) >> { >> const struct drm_mode_config *config = &dev->mode_config; >> @@ -203,16 +213,15 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane >> memcpy(formats_ptr(blob_data), plane->format_types, formats_size); >> >> /* If we can't determine support, just bail */ >> - if (!plane->funcs->format_mod_supported) >> + if (config->fb_modifiers_not_supported) >> goto done; >> >> mod = modifiers_ptr(blob_data); >> for (i = 0; i < plane->modifier_count; i++) { >> for (j = 0; j < plane->format_count; j++) { >> - if (plane->funcs->format_mod_supported(plane, >> - plane->format_types[j], >> - plane->modifiers[i])) { >> - >> + if (check_format_modifier(plane, >> + plane->format_types[j], >> + plane->modifiers[i])) { >> mod->formats |= 1ULL << j; >> } >> } >> @@ -242,6 +251,10 @@ static int __drm_universal_plane_init(struct drm_device *dev, >> const char *name, va_list ap) >> { >> struct drm_mode_config *config = &dev->mode_config; >> + const uint64_t default_modifiers[] = { >> + DRM_FORMAT_MOD_LINEAR, >> + DRM_FORMAT_MOD_INVALID >> + }; >> unsigned int format_modifier_count = 0; >> int ret; >> >> @@ -282,6 +295,11 @@ static int __drm_universal_plane_init(struct drm_device *dev, >> >> while (*temp_modifiers++ != DRM_FORMAT_MOD_INVALID) >> format_modifier_count++; >> + } else { >> + if (!dev->mode_config.fb_modifiers_not_supported) { >> + format_modifiers = default_modifiers; >> + format_modifier_count = 1; >> + } >> } >> >> /* autoset the cap and check for consistency across all planes */ >> @@ -346,7 +364,7 @@ static int __drm_universal_plane_init(struct drm_device *dev, >> drm_object_attach_property(&plane->base, config->prop_src_h, 0); >> } >> >> - if (config->allow_fb_modifiers) >> + if (format_modifier_count) >> create_in_format_blob(dev, plane); >> >> return 0; >> @@ -373,8 +391,8 @@ static int __drm_universal_plane_init(struct drm_device *dev, >> * drm_universal_plane_init() to let the DRM managed resource infrastructure >> * take care of cleanup and deallocation. >> * >> - * Drivers supporting modifiers must set @format_modifiers on all their planes, >> - * even those that only support DRM_FORMAT_MOD_LINEAR. >> + * For drivers supporting modifiers, all planes will advertise >> + * DRM_FORMAT_MOD_LINEAR support, if @format_modifiers is not set. >> * >> * Returns: >> * Zero on success, error code on failure. >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h >> index 48b7de80daf5..c56f298c55bd 100644 >> --- a/include/drm/drm_mode_config.h >> +++ b/include/drm/drm_mode_config.h >> @@ -920,6 +920,16 @@ struct drm_mode_config { >> */ >> bool allow_fb_modifiers; >> >> + /** >> + * @fb_modifiers_not_supported: >> + * >> + * This flag is for legacy drivers such as radeon that do not support > > Maybe don't put specific driver names into kerneldoc (in commit message to > motivate your changes it's fine). It's unlikely radeon ever changes on > this, but also no one will update this in the docs if we ever do that. > > Perhaps also add that new driver should never set this, just to hammer it > home that modifiers really should work everywhere. I agree with you. I'll modify this docs. Thanks, Tomohito Esaki > > Otherwise I think this series is the right thing to do. > -Daniel > >> + * modifiers but infer the actual layout of the underlying buffer. >> + * Generally, each drivers must support modifiers, this flag should not >> + * be set. >> + */ >> + bool fb_modifiers_not_supported; >> + >> /** >> * @normalize_zpos: >> * >> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h >> index 0c1102dc4d88..cad641b1f797 100644 >> --- a/include/drm/drm_plane.h >> +++ b/include/drm/drm_plane.h >> @@ -803,6 +803,9 @@ void *__drmm_universal_plane_alloc(struct drm_device *dev, >> * >> * The @drm_plane_funcs.destroy hook must be NULL. >> * >> + * For drivers supporting modifiers, all planes will advertise >> + * DRM_FORMAT_MOD_LINEAR support, if @format_modifiers is not set. >> + * >> * Returns: >> * Pointer to new plane, or ERR_PTR on failure. >> */ >> -- >> 2.17.1 >> >