Message ID | 20230623100822.274706-4-sui.jingfeng@linux.dev |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp5668133vqr; Fri, 23 Jun 2023 03:29:07 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6XB5ojdjJPSGmnNBWDM+Afrro8pVycVPK9j0TgjR3dCvmIy44dIFi+C+14azWFjAtchKiV X-Received: by 2002:a05:620a:4c92:b0:764:14bc:52a8 with SMTP id to18-20020a05620a4c9200b0076414bc52a8mr6327116qkn.9.1687516147058; Fri, 23 Jun 2023 03:29:07 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687516147; cv=none; d=google.com; s=arc-20160816; b=k2Iz0FPbibbL/Y1simmiBTR01HtvBuOWEug3EljTwKufICbvBFyQ35ium7ymmsfhtj bW8Gxy/MZWzyjKrKNB6Gx7nbyRqULoNONj1L3C8vM8ttuxUt51jgUoya73csIiQXQFIN t0yE8dWDJ374Mr08/J+tbzsU6nJqWSphUzf1Nbx3KS8OQbgy/V6K0zGK9CnvOcsU4+ww oZJZAskbOM8/TPW+zKoo0fxB6ycEbfSynVgjEeVn9QPg+2S71o8CMXWAplS6bNwNp9d+ Ht1H2DYFpmvdLM4G0bsHTtPvDK6fwujGqXq/c+zBZ4eFjBAXhvcrbBorC1N0M8WHakLG J2HQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=HapoD/NYZ/OiH+lAomI9HpFCENnwDqcm4Zh/feZWP3Y=; b=tPAqHgFjO5bxGcPt/1ybt66L1vkdvMlw/Ujgz06Zw9lQDqfLNIcwkcBydG1icXSvQH OF3ctwOp/oD8O1CYFDawuvvsQa5J1I7viuSQGIEafVlHZI5WjKrt1Qk88xmbYXM5HBvs oZOcRd0OKhXWcPskNLvyaI3JNAu7tOvA8wLw4Do87KnjiDjzzl7Cr/IIfwZSM2BI4GIu KcskQGXLBsuEN3WxgLLbBwQe99//X61arBdH4F455TjTUye/Hv8/ImhTvGoAzKU90Kdk rmnlkTqYWcWLQcQpY5r2CQljJqELY+VHAlcNCgF9QwdhgD60zPBDAe19OyaRNn/1RFy5 FH/A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.dev header.s=key1 header.b=hpR0WYsU; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.dev Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id lb14-20020a17090b4a4e00b0024e2afd72a3si1772457pjb.182.2023.06.23.03.28.54; Fri, 23 Jun 2023 03:29:07 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.dev header.s=key1 header.b=hpR0WYsU; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.dev Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232167AbjFWKRo (ORCPT <rfc822;maxin.john@gmail.com> + 99 others); Fri, 23 Jun 2023 06:17:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56734 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231646AbjFWKRQ (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 23 Jun 2023 06:17:16 -0400 Received: from out-31.mta0.migadu.com (out-31.mta0.migadu.com [IPv6:2001:41d0:1004:224b::1f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EFBD710F1 for <linux-kernel@vger.kernel.org>; Fri, 23 Jun 2023 03:17:05 -0700 (PDT) X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1687514915; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=HapoD/NYZ/OiH+lAomI9HpFCENnwDqcm4Zh/feZWP3Y=; b=hpR0WYsUXhqCELG7hJcoQpTmNanUXJvDl9+HBv27lzRFj5gYV/9ygVqGeGk2PYpyMI17Sb mu578hE5Urf/rjylw/BPFNt5mI7W9DSpQJwPQtHToFYGInI6Bl3TUnROlqqkBGJMMimOBn ncKbpVf952YfeWYqicPaDunD0BrsbHo= From: Sui Jingfeng <sui.jingfeng@linux.dev> To: Lucas Stach <l.stach@pengutronix.de>, Russell King <linux+etnaviv@armlinux.org.uk>, Christian Gmeiner <christian.gmeiner@gmail.com>, David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch> Cc: etnaviv@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, loongson-kernel@lists.loongnix.cn, Sui Jingfeng <suijingfeng@loongson.cn> Subject: [PATCH v1 3/8] drm/etnaviv: Drop the second argument of the etnaviv_gem_new_impl() Date: Fri, 23 Jun 2023 18:08:17 +0800 Message-Id: <20230623100822.274706-4-sui.jingfeng@linux.dev> In-Reply-To: <20230623100822.274706-1-sui.jingfeng@linux.dev> References: <20230623100822.274706-1-sui.jingfeng@linux.dev> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1769488931506452004?= X-GMAIL-MSGID: =?utf-8?q?1769488931506452004?= |
Series |
drm/etnaviv: Various cleanup
|
|
Commit Message
Sui Jingfeng
June 23, 2023, 10:08 a.m. UTC
From: Sui Jingfeng <suijingfeng@loongson.cn> Because it is not used by the etnaviv_gem_new_impl() function, no functional change. Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> --- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
Comments
Hi Jingfeng, Am Freitag, dem 23.06.2023 um 18:08 +0800 schrieb Sui Jingfeng: > From: Sui Jingfeng <suijingfeng@loongson.cn> > > Because it is not used by the etnaviv_gem_new_impl() function, > no functional change. > I think it would make sense to move into the opposite direction: in both callsites of etnaviv_gem_new_impl we immediately call drm_gem_object_init with the size. A better cleanup would be to make use of the size parameter and move this object init call into etnaviv_gem_new_impl. Regards, Lucas > Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> > --- > drivers/gpu/drm/etnaviv/etnaviv_gem.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c > index b5f73502e3dd..be2f459c66b5 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c > @@ -542,7 +542,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = { > .vm_ops = &vm_ops, > }; > > -static int etnaviv_gem_new_impl(struct drm_device *dev, u32 size, u32 flags, > +static int etnaviv_gem_new_impl(struct drm_device *dev, u32 flags, > const struct etnaviv_gem_ops *ops, struct drm_gem_object **obj) > { > struct etnaviv_gem_object *etnaviv_obj; > @@ -591,8 +591,7 @@ int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file, > > size = PAGE_ALIGN(size); > > - ret = etnaviv_gem_new_impl(dev, size, flags, > - &etnaviv_gem_shmem_ops, &obj); > + ret = etnaviv_gem_new_impl(dev, flags, &etnaviv_gem_shmem_ops, &obj); > if (ret) > goto fail; > > @@ -627,7 +626,7 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags, > struct drm_gem_object *obj; > int ret; > > - ret = etnaviv_gem_new_impl(dev, size, flags, ops, &obj); > + ret = etnaviv_gem_new_impl(dev, flags, ops, &obj); > if (ret) > return ret; >
Hi, Lucas Thanks for you guidance! On 2023/7/17 17:51, Lucas Stach wrote: > Hi Jingfeng, > > Am Freitag, dem 23.06.2023 um 18:08 +0800 schrieb Sui Jingfeng: >> From: Sui Jingfeng <suijingfeng@loongson.cn> >> >> Because it is not used by the etnaviv_gem_new_impl() function, >> no functional change. >> > I think it would make sense to move into the opposite direction: in > both callsites of etnaviv_gem_new_impl we immediately call > drm_gem_object_init with the size. Really? But there are multiple call path to the etnaviv_gem_new_impl() function. Code path 1 (PRIME): |- etnaviv_gem_prime_import_sg_table() |-- etnaviv_gem_new_private() |--- etnaviv_gem_new_impl(dev, size, flags, ops, &obj) |--- drm_gem_private_object_init(dev, obj, size) Code path 2 (USERPTR): |- etnaviv_gem_new_userptr() |-- etnaviv_gem_new_private() |--- etnaviv_gem_new_impl(dev, size, flags, ops, &obj) |--- drm_gem_private_object_init(dev, obj, size) Code path 3 (construct a GEM buffer object for the user-space): |- etnaviv_ioctl_gem_new() |-- etnaviv_gem_new_handle() |--- etnaviv_gem_new_impl(dev, size, flags, &etnaviv_gem_shmem_ops, &obj); |--- drm_gem_object_init(dev, obj, size); If I understand this correctly: Code path 1 is for cross device (and cross driver) buffer-sharing, Code path 2 is going to share the buffer the userspace, *Only* the code path 3 is to construct a GEM buffer object for the user-space the userspace, that is say, *only* the code path 3 need to do the backing memory allocation work for the userspace. thus it need to call drm_gem_object_init() function, which really the shmem do the backing memory allocation. The code path 1 and the code path 2 do not need the kernel space allocate the backing memory. Because they are going to share the buffer already allocated by others. thus, code path 2 and code path 3 should call drm_gem_private_object_init(), *not* the drm_gem_object_init() function. When import buffer from the a specific KMS driver, then etnaviv_gem_prime_import_sg_table() will be called. I guess you means that drm_gem_private_object_init() (not the drm_gem_object_init() function)here ? > A better cleanup would be to make > use of the size parameter and move this object init call into > etnaviv_gem_new_impl. If I following you guidance, how do I differentiate the cases when to call drm_gem_private_object_init() not drm_gem_object_init() ? and when call drm_gem_object_init() not drm_gem_private_object_init()? I don't think you are right here. > > Regards, > Lucas > >> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> >> --- >> drivers/gpu/drm/etnaviv/etnaviv_gem.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c >> index b5f73502e3dd..be2f459c66b5 100644 >> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c >> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c >> @@ -542,7 +542,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = { >> .vm_ops = &vm_ops, >> }; >> >> -static int etnaviv_gem_new_impl(struct drm_device *dev, u32 size, u32 flags, >> +static int etnaviv_gem_new_impl(struct drm_device *dev, u32 flags, >> const struct etnaviv_gem_ops *ops, struct drm_gem_object **obj) >> { >> struct etnaviv_gem_object *etnaviv_obj; >> @@ -591,8 +591,7 @@ int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file, >> >> size = PAGE_ALIGN(size); >> >> - ret = etnaviv_gem_new_impl(dev, size, flags, >> - &etnaviv_gem_shmem_ops, &obj); >> + ret = etnaviv_gem_new_impl(dev, flags, &etnaviv_gem_shmem_ops, &obj); >> if (ret) >> goto fail; >> >> @@ -627,7 +626,7 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags, >> struct drm_gem_object *obj; >> int ret; >> >> - ret = etnaviv_gem_new_impl(dev, size, flags, ops, &obj); >> + ret = etnaviv_gem_new_impl(dev, flags, ops, &obj); >> if (ret) >> return ret; >>
Hi Jingfeng, Am Dienstag, dem 18.07.2023 um 02:34 +0800 schrieb suijingfeng: > Hi, Lucas > > > Thanks for you guidance! > > > On 2023/7/17 17:51, Lucas Stach wrote: > > Hi Jingfeng, > > > > Am Freitag, dem 23.06.2023 um 18:08 +0800 schrieb Sui Jingfeng: > > > From: Sui Jingfeng <suijingfeng@loongson.cn> > > > > > > Because it is not used by the etnaviv_gem_new_impl() function, > > > no functional change. > > > > > I think it would make sense to move into the opposite direction: in > > both callsites of etnaviv_gem_new_impl we immediately call > > drm_gem_object_init with the size. > > Really? > > But there are multiple call path to the etnaviv_gem_new_impl() function. > > > Code path 1 (PRIME): > > > - etnaviv_gem_prime_import_sg_table() > > > -- etnaviv_gem_new_private() > > > --- etnaviv_gem_new_impl(dev, size, flags, ops, &obj) > > > --- drm_gem_private_object_init(dev, obj, size) > > > Code path 2 (USERPTR): > > > - etnaviv_gem_new_userptr() > > > -- etnaviv_gem_new_private() > > > --- etnaviv_gem_new_impl(dev, size, flags, ops, &obj) > > > --- drm_gem_private_object_init(dev, obj, size) > > > Code path 3 (construct a GEM buffer object for the user-space): > > > - etnaviv_ioctl_gem_new() > > > -- etnaviv_gem_new_handle() > > > --- etnaviv_gem_new_impl(dev, size, flags, &etnaviv_gem_shmem_ops, &obj); > > > --- drm_gem_object_init(dev, obj, size); > > > If I understand this correctly: > > > Code path 1 is for cross device (and cross driver) buffer-sharing, > > Code path 2 is going to share the buffer the userspace, > > > *Only* the code path 3 is to construct a GEM buffer object for the > user-space the userspace, > > that is say, *only* the code path 3 need to do the backing memory > allocation work for the userspace. > > thus it need to call drm_gem_object_init() function, which really the > shmem do the backing memory > > allocation. > > > The code path 1 and the code path 2 do not need the kernel space > allocate the backing memory. > > Because they are going to share the buffer already allocated by others. > > thus, code path 2 and code path 3 should call drm_gem_private_object_init(), > > *not* the drm_gem_object_init() function. > > > When import buffer from the a specific KMS driver, > > then etnaviv_gem_prime_import_sg_table() will be called. > > > I guess you means that drm_gem_private_object_init() (not the > drm_gem_object_init() function)here ? > > > > A better cleanup would be to make > > use of the size parameter and move this object init call into > > etnaviv_gem_new_impl. > > If I following you guidance, how do I differentiate the cases > > when to call drm_gem_private_object_init() not drm_gem_object_init() ? > > and when call drm_gem_object_init() not drm_gem_private_object_init()? > > > I don't think you are right here. > Yes, clearly I was not taking into account the differences between drm_gem_private_object_init and drm_gem_object_init properly. Please disregard my comment, this patch is good as-is. Regards, Lucas > > > > Regards, > > Lucas > > > > > Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> > > > --- > > > drivers/gpu/drm/etnaviv/etnaviv_gem.c | 7 +++---- > > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c > > > index b5f73502e3dd..be2f459c66b5 100644 > > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c > > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c > > > @@ -542,7 +542,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = { > > > .vm_ops = &vm_ops, > > > }; > > > > > > -static int etnaviv_gem_new_impl(struct drm_device *dev, u32 size, u32 flags, > > > +static int etnaviv_gem_new_impl(struct drm_device *dev, u32 flags, > > > const struct etnaviv_gem_ops *ops, struct drm_gem_object **obj) > > > { > > > struct etnaviv_gem_object *etnaviv_obj; > > > @@ -591,8 +591,7 @@ int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file, > > > > > > size = PAGE_ALIGN(size); > > > > > > - ret = etnaviv_gem_new_impl(dev, size, flags, > > > - &etnaviv_gem_shmem_ops, &obj); > > > + ret = etnaviv_gem_new_impl(dev, flags, &etnaviv_gem_shmem_ops, &obj); > > > if (ret) > > > goto fail; > > > > > > @@ -627,7 +626,7 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags, > > > struct drm_gem_object *obj; > > > int ret; > > > > > > - ret = etnaviv_gem_new_impl(dev, size, flags, ops, &obj); > > > + ret = etnaviv_gem_new_impl(dev, flags, ops, &obj); > > > if (ret) > > > return ret; > > > >
Hi, On 2023/7/18 16:12, Lucas Stach wrote: > Hi Jingfeng, > > Am Dienstag, dem 18.07.2023 um 02:34 +0800 schrieb suijingfeng: >> Hi, Lucas >> >> >> Thanks for you guidance! >> >> >> On 2023/7/17 17:51, Lucas Stach wrote: >>> Hi Jingfeng, >>> >>> Am Freitag, dem 23.06.2023 um 18:08 +0800 schrieb Sui Jingfeng: >>>> From: Sui Jingfeng <suijingfeng@loongson.cn> >>>> >>>> Because it is not used by the etnaviv_gem_new_impl() function, >>>> no functional change. >>>> >>> I think it would make sense to move into the opposite direction: in >>> both callsites of etnaviv_gem_new_impl we immediately call >>> drm_gem_object_init with the size. >> Really? >> >> But there are multiple call path to the etnaviv_gem_new_impl() function. >> >> >> Code path 1 (PRIME): >> >>> - etnaviv_gem_prime_import_sg_table() >>> -- etnaviv_gem_new_private() >>> --- etnaviv_gem_new_impl(dev, size, flags, ops, &obj) >>> --- drm_gem_private_object_init(dev, obj, size) >> >> Code path 2 (USERPTR): >> >>> - etnaviv_gem_new_userptr() >>> -- etnaviv_gem_new_private() >>> --- etnaviv_gem_new_impl(dev, size, flags, ops, &obj) >>> --- drm_gem_private_object_init(dev, obj, size) >> >> Code path 3 (construct a GEM buffer object for the user-space): >> >>> - etnaviv_ioctl_gem_new() >>> -- etnaviv_gem_new_handle() >>> --- etnaviv_gem_new_impl(dev, size, flags, &etnaviv_gem_shmem_ops, &obj); >>> --- drm_gem_object_init(dev, obj, size); >> >> If I understand this correctly: >> >> >> Code path 1 is for cross device (and cross driver) buffer-sharing, >> >> Code path 2 is going to share the buffer the userspace, >> >> >> *Only* the code path 3 is to construct a GEM buffer object for the >> user-space the userspace, >> >> that is say, *only* the code path 3 need to do the backing memory >> allocation work for the userspace. >> >> thus it need to call drm_gem_object_init() function, which really the >> shmem do the backing memory >> >> allocation. >> >> >> The code path 1 and the code path 2 do not need the kernel space >> allocate the backing memory. >> >> Because they are going to share the buffer already allocated by others. >> >> thus, code path 2 and code path 3 should call drm_gem_private_object_init(), >> >> *not* the drm_gem_object_init() function. >> >> >> When import buffer from the a specific KMS driver, >> >> then etnaviv_gem_prime_import_sg_table() will be called. >> >> >> I guess you means that drm_gem_private_object_init() (not the >> drm_gem_object_init() function)here ? >> >> >>> A better cleanup would be to make >>> use of the size parameter and move this object init call into >>> etnaviv_gem_new_impl. >> If I following you guidance, how do I differentiate the cases >> >> when to call drm_gem_private_object_init() not drm_gem_object_init() ? >> >> and when call drm_gem_object_init() not drm_gem_private_object_init()? >> >> >> I don't think you are right here. >> > Yes, clearly I was not taking into account the differences between > drm_gem_private_object_init and drm_gem_object_init properly. Please > disregard my comment, this patch is good as-is. I have study your patch in the past frequently. As you could solve very complex(and difficulty) bugs. So I still believe that you know everything about etnaviv. I'm just wondering that you are designing the traps. But I'm not sure. Okay, still acceptable. Because communicate will you is interesting. Thank you. > Regards, > Lucas > >>> Regards, >>> Lucas >>> >>>> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> >>>> --- >>>> drivers/gpu/drm/etnaviv/etnaviv_gem.c | 7 +++---- >>>> 1 file changed, 3 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c >>>> index b5f73502e3dd..be2f459c66b5 100644 >>>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c >>>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c >>>> @@ -542,7 +542,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = { >>>> .vm_ops = &vm_ops, >>>> }; >>>> >>>> -static int etnaviv_gem_new_impl(struct drm_device *dev, u32 size, u32 flags, >>>> +static int etnaviv_gem_new_impl(struct drm_device *dev, u32 flags, >>>> const struct etnaviv_gem_ops *ops, struct drm_gem_object **obj) >>>> { >>>> struct etnaviv_gem_object *etnaviv_obj; >>>> @@ -591,8 +591,7 @@ int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file, >>>> >>>> size = PAGE_ALIGN(size); >>>> >>>> - ret = etnaviv_gem_new_impl(dev, size, flags, >>>> - &etnaviv_gem_shmem_ops, &obj); >>>> + ret = etnaviv_gem_new_impl(dev, flags, &etnaviv_gem_shmem_ops, &obj); >>>> if (ret) >>>> goto fail; >>>> >>>> @@ -627,7 +626,7 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags, >>>> struct drm_gem_object *obj; >>>> int ret; >>>> >>>> - ret = etnaviv_gem_new_impl(dev, size, flags, ops, &obj); >>>> + ret = etnaviv_gem_new_impl(dev, flags, ops, &obj); >>>> if (ret) >>>> return ret; >>>>
On 2023/7/19 00:16, suijingfeng wrote:
> Because communicate will you
'will' -> 'with'
Hi Jingfeng, Am Mittwoch, dem 19.07.2023 um 00:16 +0800 schrieb suijingfeng: > Hi, > > > [...] > > > I don't think you are right here. > > > > > Yes, clearly I was not taking into account the differences between > > drm_gem_private_object_init and drm_gem_object_init properly. Please > > disregard my comment, this patch is good as-is. > > I have study your patch in the past frequently. > > As you could solve very complex(and difficulty) bugs. > > So I still believe that you know everything about etnaviv. > While flattering, even I myself am not thinking I know everything about etnaviv. The different HW generations and the complex subsystem the driver is living in doesn't make it easy for anyone to keep in mind everything. > I'm just wondering that you are designing the traps. But I'm not sure. > Certainly not. I'm just human and do make mistakes as everyone. During the quick scrolling though the code when reviewing this patch my mind clearly just dropped the _private_ part of one of the function names. I appreciate being told when I am wrong and I do believe that the exchange during the review is helpful for everyone to get on the same page. Regards, Lucas > Okay, still acceptable. > > Because communicate will you is interesting. > > Thank you. > > > Regards, > > Lucas > > > > > > Regards, > > > > Lucas > > > > > > > > > Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn> > > > > > --- > > > > > drivers/gpu/drm/etnaviv/etnaviv_gem.c | 7 +++---- > > > > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c > > > > > index b5f73502e3dd..be2f459c66b5 100644 > > > > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c > > > > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c > > > > > @@ -542,7 +542,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = { > > > > > .vm_ops = &vm_ops, > > > > > }; > > > > > > > > > > -static int etnaviv_gem_new_impl(struct drm_device *dev, u32 size, u32 flags, > > > > > +static int etnaviv_gem_new_impl(struct drm_device *dev, u32 flags, > > > > > const struct etnaviv_gem_ops *ops, struct drm_gem_object **obj) > > > > > { > > > > > struct etnaviv_gem_object *etnaviv_obj; > > > > > @@ -591,8 +591,7 @@ int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file, > > > > > > > > > > size = PAGE_ALIGN(size); > > > > > > > > > > - ret = etnaviv_gem_new_impl(dev, size, flags, > > > > > - &etnaviv_gem_shmem_ops, &obj); > > > > > + ret = etnaviv_gem_new_impl(dev, flags, &etnaviv_gem_shmem_ops, &obj); > > > > > if (ret) > > > > > goto fail; > > > > > > > > > > @@ -627,7 +626,7 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags, > > > > > struct drm_gem_object *obj; > > > > > int ret; > > > > > > > > > > - ret = etnaviv_gem_new_impl(dev, size, flags, ops, &obj); > > > > > + ret = etnaviv_gem_new_impl(dev, flags, ops, &obj); > > > > > if (ret) > > > > > return ret; > > > > > >
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c index b5f73502e3dd..be2f459c66b5 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c @@ -542,7 +542,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = { .vm_ops = &vm_ops, }; -static int etnaviv_gem_new_impl(struct drm_device *dev, u32 size, u32 flags, +static int etnaviv_gem_new_impl(struct drm_device *dev, u32 flags, const struct etnaviv_gem_ops *ops, struct drm_gem_object **obj) { struct etnaviv_gem_object *etnaviv_obj; @@ -591,8 +591,7 @@ int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file, size = PAGE_ALIGN(size); - ret = etnaviv_gem_new_impl(dev, size, flags, - &etnaviv_gem_shmem_ops, &obj); + ret = etnaviv_gem_new_impl(dev, flags, &etnaviv_gem_shmem_ops, &obj); if (ret) goto fail; @@ -627,7 +626,7 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags, struct drm_gem_object *obj; int ret; - ret = etnaviv_gem_new_impl(dev, size, flags, ops, &obj); + ret = etnaviv_gem_new_impl(dev, flags, ops, &obj); if (ret) return ret;