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 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B465FC433EF for ; Tue, 18 Jan 2022 09:54:55 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8577A112A00; Tue, 18 Jan 2022 09:54:54 +0000 (UTC) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTPS id C61CC10F2C1; Tue, 18 Jan 2022 09:54:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1642499692; x=1674035692; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=iCK4/KbCsLlMc5j0Y8eHw+xQocZMcWu5acqJbztI50s=; b=QXaYSX7T/nV3XJHXqXekcF5mSjkKBuVOPJsqfexmN7Q24I6/tYNrCXVU +LVWmUIZBolcPaQwUVQFDDSSmWftNgxAI0U1mC/0/EdcmMqksV/6LHsZC 6AdEOyQ6K8/8G2nODup6frNdc160MG4B91L6c1aAAbcZKj5nZuh8HQseo NtI2NtLT2B3NRyFgq5KbD0l/IHi1KSfsvhCJ18TlY2SXGYu0LGDSm3Ni0 Acn9MF00BsunxPVdo1d18LhQbt2CUjhKws5f9LLa/2vwAZs31M+KXbdnu 1A/xoKvs3pake1QhEWXe0CmRkJBm8qM30dbqJuTyiLfJBbnev6y0bexsj A==; X-IronPort-AV: E=McAfee;i="6200,9189,10230"; a="269158058" X-IronPort-AV: E=Sophos;i="5.88,297,1635231600"; d="scan'208";a="269158058" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jan 2022 01:54:52 -0800 X-IronPort-AV: E=Sophos;i="5.88,297,1635231600"; d="scan'208";a="595015751" Received: from smile.fi.intel.com ([10.237.72.61]) by fmsmga004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jan 2022 01:54:46 -0800 Received: from andy by smile.fi.intel.com with local (Exim 4.95) (envelope-from ) id 1n9lBG-00Bnof-PN; Tue, 18 Jan 2022 11:53:34 +0200 Date: Tue, 18 Jan 2022 11:53:34 +0200 From: Andy Shevchenko To: Esaki Tomohito Subject: Re: [RFC PATCH v3 2/3] drm: add support modifiers for drivers whose planes only support linear layout Message-ID: References: <20220114101753.24996-1-etom@igel.co.jp> <20220114101753.24996-3-etom@igel.co.jp> <0cf405a1-0d2d-ed5e-abdf-be645e7a9209@igel.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0cf405a1-0d2d-ed5e-abdf-be645e7a9209@igel.co.jp> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: David Airlie , nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org, Michel =?iso-8859-1?Q?D=E4nzer?= , Lee Jones , Rob Clark , Takanari Hayama , amd-gfx@lists.freedesktop.org, Ben Skeggs , Petr Mladek , Sakari Ailus , Abhinav Kumar , Alex Deucher , Sean Paul , Evan Quan , Mark Yacoub , Qingqing Zhuo , "Pan, Xinhui" , linux-kernel@vger.kernel.org, Thomas Zimmermann , Dmitry Baryshkov , Damian Hobson-Garcia , Christian =?iso-8859-1?Q?K=F6nig?= , Laurent Pinchart Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Mon, Jan 17, 2022 at 02:15:48PM +0900, Esaki Tomohito wrote: > On 2022/01/14 23:16, Andy Shevchenko wrote: > > On Fri, Jan 14, 2022 at 07:17:52PM +0900, Tomohito Esaki wrote: > > > The LINEAR modifier is advertised as default if a driver doesn't specify > > > modifiers. > > > > ... > > > > > + const uint64_t default_modifiers[] = { > > > + DRM_FORMAT_MOD_LINEAR, > > > + DRM_FORMAT_MOD_INVALID > > > > + Comma? > > There is no mention in the coding style about adding/removing a comma to the > last element of an array. Is there a policy in drm driver? > > I think the advantage of adding a comma to the last element of an array is > that diff is only one line when an element is added to the end. > However since INVALID is always the last element in the modifiers array, I > think it can be either in this case. > If there is a policy, I will match it. Indeed, but there is a common sense. The idea behind (multi-line) definitions that when next time somebody will add an element in the array, there are will be: a) no additional churn (like in case of this patch, if the item will be added at the bottom; b) an element that may not be added behind the terminator, which will look weird. That said, the question is if the element is terminator one or not, if not, comma is better than no comma and vise versa. -- With Best Regards, Andy Shevchenko 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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A0C4AC433EF for ; Tue, 18 Jan 2022 09:55:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234332AbiARJzD (ORCPT ); Tue, 18 Jan 2022 04:55:03 -0500 Received: from mga04.intel.com ([192.55.52.120]:10568 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235911AbiARJyy (ORCPT ); Tue, 18 Jan 2022 04:54:54 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1642499694; x=1674035694; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=iCK4/KbCsLlMc5j0Y8eHw+xQocZMcWu5acqJbztI50s=; b=k3jgwShbIZUiAiw1s/gcq6ZZB8KZ2t420StYpNyCMDcyx2+wl+Notlcc BPszJMqFA7vvH8ukwJpb6neQmhGHusLCWKNbv4tpJeXEEvUF/P0Q4lwPm /K9S7I8eIYTfzSMJfr7GqnYiaAzDrJmyH3gZqKzt/nRslMnR9pRxeHLne gUTBlctvJLkU1Jvg+IBa/IhOgC4BbVCj+smIJtzcWlTKS8UuA+y9oFaZN HAYSP8OM4nLX5jBrsLNPlxsOMlJn+uikpDi5aZ/kLC19azDLI595rM3HB 1OhVv/+veFPcwj/dJrp24Byngw9aPAaPWKHwmpWA7bs3hmMTuiEXnF73K A==; X-IronPort-AV: E=McAfee;i="6200,9189,10230"; a="243602768" X-IronPort-AV: E=Sophos;i="5.88,297,1635231600"; d="scan'208";a="243602768" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jan 2022 01:54:52 -0800 X-IronPort-AV: E=Sophos;i="5.88,297,1635231600"; d="scan'208";a="595015751" Received: from smile.fi.intel.com ([10.237.72.61]) by fmsmga004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jan 2022 01:54:46 -0800 Received: from andy by smile.fi.intel.com with local (Exim 4.95) (envelope-from ) id 1n9lBG-00Bnof-PN; Tue, 18 Jan 2022 11:53:34 +0200 Date: Tue, 18 Jan 2022 11:53:34 +0200 From: Andy Shevchenko To: Esaki Tomohito Cc: dri-devel@lists.freedesktop.org, Alex Deucher , Christian =?iso-8859-1?Q?K=F6nig?= , "Pan, Xinhui" , David Airlie , Daniel Vetter , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Ben Skeggs , Michel =?iso-8859-1?Q?D=E4nzer?= , Simon Ser , Qingqing Zhuo , Bas Nieuwenhuizen , Mark Yacoub , Sean Paul , Evan Quan , 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, Daniel Stone , Laurent Pinchart , Damian Hobson-Garcia , Takanari Hayama Subject: Re: [RFC PATCH v3 2/3] drm: add support modifiers for drivers whose planes only support linear layout Message-ID: References: <20220114101753.24996-1-etom@igel.co.jp> <20220114101753.24996-3-etom@igel.co.jp> <0cf405a1-0d2d-ed5e-abdf-be645e7a9209@igel.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0cf405a1-0d2d-ed5e-abdf-be645e7a9209@igel.co.jp> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jan 17, 2022 at 02:15:48PM +0900, Esaki Tomohito wrote: > On 2022/01/14 23:16, Andy Shevchenko wrote: > > On Fri, Jan 14, 2022 at 07:17:52PM +0900, Tomohito Esaki wrote: > > > The LINEAR modifier is advertised as default if a driver doesn't specify > > > modifiers. > > > > ... > > > > > + const uint64_t default_modifiers[] = { > > > + DRM_FORMAT_MOD_LINEAR, > > > + DRM_FORMAT_MOD_INVALID > > > > + Comma? > > There is no mention in the coding style about adding/removing a comma to the > last element of an array. Is there a policy in drm driver? > > I think the advantage of adding a comma to the last element of an array is > that diff is only one line when an element is added to the end. > However since INVALID is always the last element in the modifiers array, I > think it can be either in this case. > If there is a policy, I will match it. Indeed, but there is a common sense. The idea behind (multi-line) definitions that when next time somebody will add an element in the array, there are will be: a) no additional churn (like in case of this patch, if the item will be added at the bottom; b) an element that may not be added behind the terminator, which will look weird. That said, the question is if the element is terminator one or not, if not, comma is better than no comma and vise versa. -- With Best Regards, Andy Shevchenko 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 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 35575C433F5 for ; Tue, 18 Jan 2022 09:54:54 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9F03A10F2C1; Tue, 18 Jan 2022 09:54:53 +0000 (UTC) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTPS id C61CC10F2C1; Tue, 18 Jan 2022 09:54:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1642499692; x=1674035692; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=iCK4/KbCsLlMc5j0Y8eHw+xQocZMcWu5acqJbztI50s=; b=QXaYSX7T/nV3XJHXqXekcF5mSjkKBuVOPJsqfexmN7Q24I6/tYNrCXVU +LVWmUIZBolcPaQwUVQFDDSSmWftNgxAI0U1mC/0/EdcmMqksV/6LHsZC 6AdEOyQ6K8/8G2nODup6frNdc160MG4B91L6c1aAAbcZKj5nZuh8HQseo NtI2NtLT2B3NRyFgq5KbD0l/IHi1KSfsvhCJ18TlY2SXGYu0LGDSm3Ni0 Acn9MF00BsunxPVdo1d18LhQbt2CUjhKws5f9LLa/2vwAZs31M+KXbdnu 1A/xoKvs3pake1QhEWXe0CmRkJBm8qM30dbqJuTyiLfJBbnev6y0bexsj A==; X-IronPort-AV: E=McAfee;i="6200,9189,10230"; a="269158058" X-IronPort-AV: E=Sophos;i="5.88,297,1635231600"; d="scan'208";a="269158058" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jan 2022 01:54:52 -0800 X-IronPort-AV: E=Sophos;i="5.88,297,1635231600"; d="scan'208";a="595015751" Received: from smile.fi.intel.com ([10.237.72.61]) by fmsmga004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jan 2022 01:54:46 -0800 Received: from andy by smile.fi.intel.com with local (Exim 4.95) (envelope-from ) id 1n9lBG-00Bnof-PN; Tue, 18 Jan 2022 11:53:34 +0200 Date: Tue, 18 Jan 2022 11:53:34 +0200 From: Andy Shevchenko To: Esaki Tomohito Message-ID: References: <20220114101753.24996-1-etom@igel.co.jp> <20220114101753.24996-3-etom@igel.co.jp> <0cf405a1-0d2d-ed5e-abdf-be645e7a9209@igel.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0cf405a1-0d2d-ed5e-abdf-be645e7a9209@igel.co.jp> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo Subject: Re: [Nouveau] [RFC PATCH v3 2/3] drm: add support modifiers for drivers whose planes only support linear layout X-BeenThere: nouveau@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Nouveau development list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: David Airlie , nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org, Michel =?iso-8859-1?Q?D=E4nzer?= , Daniel Stone , Lee Jones , Rob Clark , Takanari Hayama , amd-gfx@lists.freedesktop.org, Ben Skeggs , Petr Mladek , Sakari Ailus , Bas Nieuwenhuizen , Maarten Lankhorst , Abhinav Kumar , Alex Deucher , Sean Paul , Maxime Ripard , Daniel Vetter , Evan Quan , Mark Yacoub , Qingqing Zhuo , "Pan, Xinhui" , linux-kernel@vger.kernel.org, Simon Ser , Dmitry Baryshkov , Damian Hobson-Garcia , Christian =?iso-8859-1?Q?K=F6nig?= , Laurent Pinchart Errors-To: nouveau-bounces@lists.freedesktop.org Sender: "Nouveau" On Mon, Jan 17, 2022 at 02:15:48PM +0900, Esaki Tomohito wrote: > On 2022/01/14 23:16, Andy Shevchenko wrote: > > On Fri, Jan 14, 2022 at 07:17:52PM +0900, Tomohito Esaki wrote: > > > The LINEAR modifier is advertised as default if a driver doesn't specify > > > modifiers. > > > > ... > > > > > + const uint64_t default_modifiers[] = { > > > + DRM_FORMAT_MOD_LINEAR, > > > + DRM_FORMAT_MOD_INVALID > > > > + Comma? > > There is no mention in the coding style about adding/removing a comma to the > last element of an array. Is there a policy in drm driver? > > I think the advantage of adding a comma to the last element of an array is > that diff is only one line when an element is added to the end. > However since INVALID is always the last element in the modifiers array, I > think it can be either in this case. > If there is a policy, I will match it. Indeed, but there is a common sense. The idea behind (multi-line) definitions that when next time somebody will add an element in the array, there are will be: a) no additional churn (like in case of this patch, if the item will be added at the bottom; b) an element that may not be added behind the terminator, which will look weird. That said, the question is if the element is terminator one or not, if not, comma is better than no comma and vise versa. -- With Best Regards, Andy Shevchenko 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 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id EAAE5C433F5 for ; Tue, 18 Jan 2022 14:19:34 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5794D10E55D; Tue, 18 Jan 2022 14:19:31 +0000 (UTC) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTPS id C61CC10F2C1; Tue, 18 Jan 2022 09:54:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1642499692; x=1674035692; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=iCK4/KbCsLlMc5j0Y8eHw+xQocZMcWu5acqJbztI50s=; b=QXaYSX7T/nV3XJHXqXekcF5mSjkKBuVOPJsqfexmN7Q24I6/tYNrCXVU +LVWmUIZBolcPaQwUVQFDDSSmWftNgxAI0U1mC/0/EdcmMqksV/6LHsZC 6AdEOyQ6K8/8G2nODup6frNdc160MG4B91L6c1aAAbcZKj5nZuh8HQseo NtI2NtLT2B3NRyFgq5KbD0l/IHi1KSfsvhCJ18TlY2SXGYu0LGDSm3Ni0 Acn9MF00BsunxPVdo1d18LhQbt2CUjhKws5f9LLa/2vwAZs31M+KXbdnu 1A/xoKvs3pake1QhEWXe0CmRkJBm8qM30dbqJuTyiLfJBbnev6y0bexsj A==; X-IronPort-AV: E=McAfee;i="6200,9189,10230"; a="269158058" X-IronPort-AV: E=Sophos;i="5.88,297,1635231600"; d="scan'208";a="269158058" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jan 2022 01:54:52 -0800 X-IronPort-AV: E=Sophos;i="5.88,297,1635231600"; d="scan'208";a="595015751" Received: from smile.fi.intel.com ([10.237.72.61]) by fmsmga004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jan 2022 01:54:46 -0800 Received: from andy by smile.fi.intel.com with local (Exim 4.95) (envelope-from ) id 1n9lBG-00Bnof-PN; Tue, 18 Jan 2022 11:53:34 +0200 Date: Tue, 18 Jan 2022 11:53:34 +0200 From: Andy Shevchenko To: Esaki Tomohito Subject: Re: [RFC PATCH v3 2/3] drm: add support modifiers for drivers whose planes only support linear layout Message-ID: References: <20220114101753.24996-1-etom@igel.co.jp> <20220114101753.24996-3-etom@igel.co.jp> <0cf405a1-0d2d-ed5e-abdf-be645e7a9209@igel.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0cf405a1-0d2d-ed5e-abdf-be645e7a9209@igel.co.jp> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo X-Mailman-Approved-At: Tue, 18 Jan 2022 14:19:29 +0000 X-BeenThere: amd-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: David Airlie , nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org, Michel =?iso-8859-1?Q?D=E4nzer?= , Daniel Stone , Lee Jones , Rob Clark , Takanari Hayama , amd-gfx@lists.freedesktop.org, Ben Skeggs , Petr Mladek , Sakari Ailus , Bas Nieuwenhuizen , Maarten Lankhorst , Abhinav Kumar , Alex Deucher , Sean Paul , Maxime Ripard , Daniel Vetter , Evan Quan , Mark Yacoub , Qingqing Zhuo , "Pan, Xinhui" , linux-kernel@vger.kernel.org, Thomas Zimmermann , Simon Ser , Dmitry Baryshkov , Damian Hobson-Garcia , Christian =?iso-8859-1?Q?K=F6nig?= , Laurent Pinchart Errors-To: amd-gfx-bounces@lists.freedesktop.org Sender: "amd-gfx" On Mon, Jan 17, 2022 at 02:15:48PM +0900, Esaki Tomohito wrote: > On 2022/01/14 23:16, Andy Shevchenko wrote: > > On Fri, Jan 14, 2022 at 07:17:52PM +0900, Tomohito Esaki wrote: > > > The LINEAR modifier is advertised as default if a driver doesn't specify > > > modifiers. > > > > ... > > > > > + const uint64_t default_modifiers[] = { > > > + DRM_FORMAT_MOD_LINEAR, > > > + DRM_FORMAT_MOD_INVALID > > > > + Comma? > > There is no mention in the coding style about adding/removing a comma to the > last element of an array. Is there a policy in drm driver? > > I think the advantage of adding a comma to the last element of an array is > that diff is only one line when an element is added to the end. > However since INVALID is always the last element in the modifiers array, I > think it can be either in this case. > If there is a policy, I will match it. Indeed, but there is a common sense. The idea behind (multi-line) definitions that when next time somebody will add an element in the array, there are will be: a) no additional churn (like in case of this patch, if the item will be added at the bottom; b) an element that may not be added behind the terminator, which will look weird. That said, the question is if the element is terminator one or not, if not, comma is better than no comma and vise versa. -- With Best Regards, Andy Shevchenko