Received-SPF: pass (google.com: domain of linux-kernel+bounces-196943-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <955a6d94-9a4c-4f7d-974c-819261579f14@linux.dev>
Date: Fri, 31 May 2024 22:04:29 +0800
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [v4,1/2] drm/bridge: sii902x: Fix mode_valid hook
To: Sam Ravnborg <sam@ravnborg.org>, Jayesh Choudhary <j-choudhary@ti.com>
Cc: linux-kernel@vger.kernel.org, dmitry.baryshkov@linaro.org,
 andrzej.hajda@intel.com, neil.armstrong@linaro.org, rfoss@kernel.org,
 Laurent.pinchart@ideasonboard.com, mripard@kernel.org, jonas@kwiboo.se,
 jernej.skrabec@gmail.com, maarten.lankhorst@linux.intel.com,
 tzimmermann@suse.de, airlied@gmail.com, daniel@ffwll.ch, a-bhatia1@ti.com,
 dri-devel@lists.freedesktop.org
References: <20240530092930.434026-2-j-choudhary@ti.com>
 <e5ce13e6-1007-41c9-bedc-2045d6f75480@linux.dev>
 <20240531133324.GA1715839@ravnborg.org>
Content-Language: en-US, en-AU
From: Sui Jingfeng <sui.jingfeng@linux.dev>
In-Reply-To: <20240531133324.GA1715839@ravnborg.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Hi, Jayesh


On 5/31/24 21:33, Sam Ravnborg wrote:
> Hi Jayesh,
> 
>>> +
>>>    static const struct drm_bridge_funcs sii902x_bridge_funcs = {
>>>    	.attach = sii902x_bridge_attach,
>>>    	.mode_set = sii902x_bridge_mode_set,
>>> @@ -516,6 +529,7 @@ static const struct drm_bridge_funcs sii902x_bridge_funcs = {
>>>    	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
>>>    	.atomic_get_input_bus_fmts = sii902x_bridge_atomic_get_input_bus_fmts,
>>>    	.atomic_check = sii902x_bridge_atomic_check,
>>> +	.mode_valid = sii902x_bridge_mode_valid,
> 
> As you have the possibility to test the driver, it would be nice with a
> follow-up patch that replaces the use of enable() / disable() with the
> atomic counterparts.
> 
> enable() / disable() are deprecated, so it is nice to reduce their use.

I agree with Sam.

Please using atomic uniformally with a follow-up patch, the mixed
using of atomic API and non atomic API is a little bit confusing IMO.


> 	Sam