Received-SPF: pass (google.com: domain of linux-kernel+bounces-177180-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <c1a69a18-4db2-4625-80c2-a7536347e15f@nxp.com>
Date: Mon, 13 May 2024 13:56:40 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] drm/bridge: imx: Remove redundant checks on existence of
 bridge->encoder
To: Sui Jingfeng <sui.jingfeng@linux.dev>, Shawn Guo <shawnguo@kernel.org>,
 Sascha Hauer <s.hauer@pengutronix.de>, Fabio Estevam <festevam@gmail.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>,
 Maxime Ripard <mripard@kernel.org>, Andrzej Hajda <andrzej.hajda@intel.com>,
 Neil Armstrong <neil.armstrong@linaro.org>, Robert Foss <rfoss@kernel.org>,
 Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
 Jonas Karlman <jonas@kwiboo.se>, Jernej Skrabec <jernej.skrabec@gmail.com>,
 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
 Thomas Zimmermann <tzimmermann@suse.de>, dri-devel@lists.freedesktop.org,
 linux-kernel@vger.kernel.org
References: <20240511150816.326846-1-sui.jingfeng@linux.dev>
Content-Language: en-US
From: Liu Ying <victor.liu@nxp.com>
In-Reply-To: <20240511150816.326846-1-sui.jingfeng@linux.dev>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
 =?utf-8?B?bXVMUGVRTFhIcUV6TDYySGFRcGxML0p3MnQ4cHJnQ3ZqaTZTakFjaVFONnk3?=
 =?utf-8?B?dVgrY05IQ3hsU1ZsT3pJNG15Yk1OcDlLdzlOcG5IaFFZa2o5NWM1VlY2U1FX?=
 =?utf-8?B?bUFVdjdBQTlqRHF6aXhpRnhjb0laYm1Fd3lGR0FGRWYrWk9QdXM5VDlyWk03?=
 =?utf-8?B?Mjh2cHgxZExzM0tWSFJKS1ArcGFoSWJ3RU14NmJzbER0eWRwK0pqVHh5YjV6?=
 =?utf-8?B?Tyt1T3U2b3FNY0VpVVRjYVE3NytBOWV0MDh4eHhFNTJ3QjBzNzlsYlNScFl4?=
 =?utf-8?B?Rk5tNTh1c0kwQ3NLTDA4ZEFFbFlTOVQ1T2FTa29qd3dDdE94Nk5LNzUxWVZO?=
 =?utf-8?B?aDBHa1I5OTlpTkM4cnl5TXRRcFJJU1FEbExUNlpVQ1hjNWZsaStUOHF3Mmh6?=
 =?utf-8?B?d0M4dHV4OWZueEF0Q29WTVRnZWVsWkU2Mi9QSU5rcTl3Z3JUZ3o1RFlZOW5t?=
 =?utf-8?B?Q1grUmd0TnlOeUZCYjdPbmQxME84OXV3OTd0RE5Hd21TNjllWGx1bFU5eE92?=
 =?utf-8?B?dEwxdzdQaDlScjd1ZjhzbzFEWm15bWd5Rk5INFVsWEp6YzJtNEZPQUNqRUxJ?=
 =?utf-8?B?UDR6NlFzV1hrMy8rQkJLcWdqU3lCNG1TL2pzV3c2QmJvMDNPcFdUcWN3d3VF?=
 =?utf-8?B?NmdpQ1ZqcStFem5iTDY3Vld2TklQYS9sSldpN1NBcWFMbmxZTmtzM3ZtVVRG?=
 =?utf-8?B?RDVWYTdXL29Mb3ZVZklvWW5lZWIvUHRmK2NWMG5GRS9lNklzNVdENnd1WFNZ?=
 =?utf-8?B?V05hbCsrVWdNVlZhVkROblU4NGpDWWYva3hKc0dnVEMwdHZqSFErNEZiYlVB?=
 =?utf-8?B?MFNUYlBvRGRaUWZ4a2dzdXlzV0JLZnBSb3AzV3RXY0dhTXlXdXlSSy9NZmJ5?=
 =?utf-8?B?dkd3SVBNTDdtcEk1Y0JxZVVWNzNPc29FQVFZY25LMkFzMU1COWMzbGhwd0dY?=
 =?utf-8?B?aE45dFZrelcyZXd2NEZ6eE42eWNhNW5kRXlUYmtaTmI2NTY2ZysvemoybDRS?=
 =?utf-8?B?aWtVSWI0MUFNUk12R2pzMUlkekg4RnBmN09hU21hVmswR0dLRWlnTE5icjRo?=
 =?utf-8?B?azRVbGFxWDkvaVA2YXppa1pHTHZBTWwvamxzQVZ0eGxoMTF2ZWxLM1FIZGRl?=
 =?utf-8?B?RE0zRkRKNEY0Y2p3cXRlUWRmdm5jVTNYRnJjTGY3T3A3dGltSElLNFhnVlU2?=
 =?utf-8?B?cWR2MEY5dityVisvZm50TGo0Y3BxVGFDL3hEVWJ6K2lobEhTQUZ4YkQvejhR?=
 =?utf-8?B?SjZQenRHUnBMaG9pNVBZeCsvRXFidkJHdk1uTm1LL2VkR3pHMzFBVzg4Z0Fi?=
 =?utf-8?B?TGRNSFRUTTVOQWpKRDdlNWdmN2FVWlV0T3VxTG0zWUJ1QmFUY3dKMlIraXo2?=
 =?utf-8?B?ek5KcDIydDZXL2lRT3orazlIUWdzMFVGdWxyeW1rNEt1TFp2UHNXZXpTSm9V?=
 =?utf-8?B?NUs0QS9jZEs3TmJVMjlVbDdaNStRTkJBaVNVNi9VQkRRNUVvcGlvR09NNzE4?=
 =?utf-8?B?QTZ4dVRLRFc3WjFPQjZqMHhucTdSWlZ1aXFDMWVYOE5ZRm1EeVZYQ1BxaUp0?=
 =?utf-8?B?TDBtc1h5U0ZMZmpWYS9kVWxFOHNHd0djRW1sL3ZMMjJyQnVWMDR2RjUzeDd3?=
 =?utf-8?B?V01FOFFhbkFVdW5obXQ2Z0tmNGNxdVVNWVJ0YitRbHBxOGRtdWVQWGl0aXll?=
 =?utf-8?B?cWFjNCtkZ0tRY3N4dTJYR1kza0JZWFRaWDJPR0F1eU1nQkJrb1Z4Nm45V2Fs?=
 =?utf-8?B?YU5BMVJPYm55d1VtNmNDRWY3UGJ3TzBiVDc4eVM3SEFETGpsYzlHY2VNU1dI?=
 =?utf-8?B?WklHUGtYdllWYS9yTDlWT3VqWDlGbkF4NEtmSHo4b25uRGprVDJ6elhYVldJ?=
 =?utf-8?B?UlB2dFR3RkI1cFJTZGUwN3hWVjZhdEJSVlpETmcvSmhOMHBTSlRnTUJrOXJo?=
 =?utf-8?B?RTZreDlMNDQ0ZDJqbGE3cDhQVWViT1BOdGM0eGNHODNIbjZPRjJTRTIrRnJt?=
 =?utf-8?B?aW9EMSt6TUtzcUZNa3lHaFYxYkRPZy92V1VtWTFSc1RDL1RQcTQ0Y2ZuWnF5?=
 =?utf-8?B?bXM1ZVl1SGY4N2FER3k1L2daNDgvaXZHWit4VUg3Z2xWWmpQRjFvOXg3cnVH?=
 =?utf-8?Q?rP67eQc+R5HXZvnDPDppwyAgo?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0ae3c044-4b2c-49e4-c39a-08dc73116bdd
X-MS-Exchange-CrossTenant-AuthSource: AM7PR04MB7046.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 May 2024 05:56:24.3905
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: rf1XQCkUFNjknnCSlJE/FiaCK0g8ZO6syUQvkCelxOrB1Xv9YW9oXV1O+vk5yct+I9dA2+TXmC8QMx6EiVDkSg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PA4PR04MB7839

On 5/11/24 23:08, Sui Jingfeng wrote:
> The check on the existence of bridge->encoder on the implementation layer
> of drm bridge driver is not necessary, as it has already been done in the
> drm_bridge_attach() function. It is guaranteed that the .encoder member
> of the drm_bridge instance is not NULL when various imx_xxx_bridge_attach()
> function gets called.

Nit:
ldb_bridge_attach_helper() doesn't follow the fashion of
imx_xxx_bridge_attach(), not even the other bridge attach
functions touched by this patch do.  Maybe, reword as
"when various i.MX specific bridge attach functions are
called."

Regards,
Liu Ying

> 
> Remove the redundant checking codes "if (!bridge->encoder) { ... }".
> 
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
>  drivers/gpu/drm/bridge/imx/imx-ldb-helper.c         | 5 -----
>  drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c | 5 -----
>  drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c     | 5 -----
>  drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c        | 5 -----
>  4 files changed, 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
> index 6967325cd8ee..9b5bebbe357d 100644
> --- a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
> +++ b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
> @@ -116,11 +116,6 @@ int ldb_bridge_attach_helper(struct drm_bridge *bridge,
>  		return -EINVAL;
>  	}
>  
> -	if (!bridge->encoder) {
> -		DRM_DEV_ERROR(ldb->dev, "missing encoder\n");
> -		return -ENODEV;
> -	}
> -
>  	return drm_bridge_attach(bridge->encoder,
>  				ldb_ch->next_bridge, bridge,
>  				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c
> index d0868a6ac6c9..e6dbbdc87ce2 100644
> --- a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c
> @@ -119,11 +119,6 @@ static int imx8qxp_pc_bridge_attach(struct drm_bridge *bridge,
>  		return -EINVAL;
>  	}
>  
> -	if (!bridge->encoder) {
> -		DRM_DEV_ERROR(pc->dev, "missing encoder\n");
> -		return -ENODEV;
> -	}
> -
>  	return drm_bridge_attach(bridge->encoder,
>  				 ch->next_bridge, bridge,
>  				 DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c
> index ed8b7a4e0e11..1d11cc1df43c 100644
> --- a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c
> @@ -138,11 +138,6 @@ static int imx8qxp_pixel_link_bridge_attach(struct drm_bridge *bridge,
>  		return -EINVAL;
>  	}
>  
> -	if (!bridge->encoder) {
> -		DRM_DEV_ERROR(pl->dev, "missing encoder\n");
> -		return -ENODEV;
> -	}
> -
>  	return drm_bridge_attach(bridge->encoder,
>  				 pl->next_bridge, bridge,
>  				 DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
> index 4a886cb808ca..fb7cf4369bb8 100644
> --- a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
> @@ -58,11 +58,6 @@ static int imx8qxp_pxl2dpi_bridge_attach(struct drm_bridge *bridge,
>  		return -EINVAL;
>  	}
>  
> -	if (!bridge->encoder) {
> -		DRM_DEV_ERROR(p2d->dev, "missing encoder\n");
> -		return -ENODEV;
> -	}
> -
>  	return drm_bridge_attach(bridge->encoder,
>  				 p2d->next_bridge, bridge,
>  				 DRM_BRIDGE_ATTACH_NO_CONNECTOR);