Received-SPF: pass (google.com: domain of linux-kernel+bounces-183382-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <5efe29a7-441f-4d70-b17c-5327c0a9f600@nxp.com>
Date: Mon, 20 May 2024 11:29:42 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] drm/bridge: adv7511: Exit interrupt handling when
 necessary
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
 andrzej.hajda@intel.com, neil.armstrong@linaro.org, rfoss@kernel.org,
 Laurent.pinchart@ideasonboard.com, jonas@kwiboo.se,
 jernej.skrabec@gmail.com, maarten.lankhorst@linux.intel.com,
 mripard@kernel.org, tzimmermann@suse.de, airlied@gmail.com, daniel@ffwll.ch,
 biju.das.jz@bp.renesas.com, u.kleine-koenig@pengutronix.de,
 aford173@gmail.com, jani.nikula@intel.com, bli@bang-olufsen.dk,
 sui.jingfeng@linux.dev
References: <20240516101006.2388767-1-victor.liu@nxp.com>
 <evnxopqt5be56vxuxqdj4l3mcd5nrrvucio7jhwgg4emraai3t@gherubdynyip>
Content-Language: en-US
From: Liu Ying <victor.liu@nxp.com>
In-Reply-To: <evnxopqt5be56vxuxqdj4l3mcd5nrrvucio7jhwgg4emraai3t@gherubdynyip>
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?VEZPaDBwMnAyNWIzV3FYQzdJeGVPWVRJeEVTd3h1NFpsMkxUd3FYdk9jeEFm?=
 =?utf-8?B?MWxhcDlZZmFRZU5QWWNXOStYa2xZN1lLaHptY2czNnFiQldvdm5DeVRYcHRT?=
 =?utf-8?B?N0ZkQnZEcEpMcDFzR1MrTE1yeHNFemRBczdJYmtGNEFhOFB3MnNaL1c5RU1B?=
 =?utf-8?B?Mkx6UUZNSjNsZzVRWlg1a1pVVnc2eExTQUR2K1g2UUVibG1TWGQ0VmJpUnpa?=
 =?utf-8?B?czhXeFdDRUR3d2p4c0hxRUZuazlhSHBpZ2lxOTczZFlucm9ZdlpneFdFdXYv?=
 =?utf-8?B?S3dBWGZGZTZMd0hjT2JIcGdmNDFyTjdUdUZla1VFNXdPMFlzbjlkQjY4ek5r?=
 =?utf-8?B?bXdKQlBGZ1B2N215cmRKWVlNQ0U2R1N2Z0EzdzQwN2xNaXFySStGNTdBSDFp?=
 =?utf-8?B?dW5Pdk1EZE5GQWpVQncrTVJkZVFuZ0ppakJDcFFqaU9PWmdqNnZIYTBjd2RN?=
 =?utf-8?B?WC9jTnhaU1A2UkNWMGs1ZW04K3hzOVlicGZWaTd0bTNISkFrWnFNcjlkSWtE?=
 =?utf-8?B?WWJ0WDR5QnBkbklZN3E2dWJoRnFVL09RRUxXeVhNNjkxRDRvZnZNRkY4QktZ?=
 =?utf-8?B?RFg1MC9MK1FIRVZwV3Z1RS9qeEFkaGZjWmdVQXpYR1lLT0xFVjlsdmxBQmJS?=
 =?utf-8?B?WndMNjdTTXFLbTZoTGw4QjE3RGowT3h6Y3VjeUZDYllnMGl3M29udFk0SFN1?=
 =?utf-8?B?OEd2UytrK0pLR0o4aGhuY2FYNWhKbnkrcjNHY2RxRnZqYTdRYU9MSHRQc2cz?=
 =?utf-8?B?YlRFeGNLRkVGR0pGUHlHa3d2cjNpSjlsZ3lrelU2cjc0UEYyU2RlRnNlaFEv?=
 =?utf-8?B?d1hHUDNaZ0s0ZmhsZ2FONzNLc2kxZEN3andtNnVrSDQzeWNkMGtQeTBXZlNB?=
 =?utf-8?B?SVV3M0ZJb2Z0OUV3MWw1TmVsZUNUTnpSTU54dkJyRGhJTWZIUE1sajV4MmFQ?=
 =?utf-8?B?c3dNYUhYTXpDTnVSck80cCtrTTZyZmpXb3lWeVpndERTUEI5emU1MUljVjhx?=
 =?utf-8?B?TWxCTGNIMVhQOStwNWdGLzNLTm1MMGNvYmdRbUhnZ3lFckpxYUNSQ3ljOG5r?=
 =?utf-8?B?cVlmT3k4aDIzWkJLWjE1c3lVMDVtc01TTjYvL2NZN3FiNUF4dVYvajdmSk9k?=
 =?utf-8?B?Q2NXeDJVby9sQzIzZk9ud2MwbkFmWDZEcGp3MGpBckk5NXZRcXQzNExsZTlN?=
 =?utf-8?B?aG5HY0NKRGdnL2RZMElMckJyV05tN0dWU2IzWlJBYXpOV1lUeklkZGJwSXo3?=
 =?utf-8?B?RFR1aWNhTWxIdHFRazRYZmxabmltNHBlOENmdkRXQlRMM1V6QnpYVk01alVk?=
 =?utf-8?B?bGFORGNmZDdhTUFTYTMya0xUclZUOWFLcGZSa1lCT21vYTNxVGw5SVR1M01Z?=
 =?utf-8?B?elBJWEFtWW5PQkl3cEdsZHp4V282Vm4yVkhvNnRtT0FENUtDTzNwamJ2aTkv?=
 =?utf-8?B?SGdxV0FuUUR1WGN3L290TnhKZlh3NFYwT3dCWVJtSllTTXpYUmw3U0ZHTFNI?=
 =?utf-8?B?U1NqcVAzMk1pVno0UkEyaTVJQitCNncwUUxYUkEvRXltTlBNeVJCbGwxRlcw?=
 =?utf-8?B?VThnMmdTMnN2Vk93OEowV3BheFhpcDBnc1hoQnAwclc1TWNQaGlBNldjZlNT?=
 =?utf-8?B?cDdBWG1vN2hDV3RRY050bUFJSDFRRFdqTFN4VDduK2tXMDNUUGhFSnRLT0gw?=
 =?utf-8?B?MGZhZldZTmV2WXM4N2loU1dvMEh5WlFrSUcyUjJReG0yOFZnTG5QWGpERzBL?=
 =?utf-8?B?enpKdDY5M1NRN0dOSlFsOS96UU9ZU2JlLzVmUXdHK25vVGZRb1VaMVVHbklo?=
 =?utf-8?B?WGFYRkFzK0VRK2ZSa29KSWhXOTdpcmtHRS96UDYzbzJvTGJxYXVkWFNySjZy?=
 =?utf-8?B?UFNlZ2oxRFc4dmtKeFpXS0ZCWmVZejdwSVZNbVV2MFUvZFRDMDFJSTV2b0s1?=
 =?utf-8?B?RVo3VEV2UjV3TEJGdHJqb2RJZ2h3azBaY1A4cXE3SzJHUExZN2g4bnhyRFNV?=
 =?utf-8?B?cEpLb1FEek5kbnBHVGg2OFBCV21HL1pzYUZkSGtGQWY0L0xvUEVXWExXd3M1?=
 =?utf-8?B?OEZkaTRpUlZ1ZDgrOFBKSDNnTkE4ek04bWJYZnpoN20zajlpZVQrUmk5bll4?=
 =?utf-8?Q?DyaEQVhKTeo58zjZ59EPqLxcu?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ba43ba4c-fd41-40dc-2c87-08dc787d0c3f
X-MS-Exchange-CrossTenant-AuthSource: AM7PR04MB7046.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 May 2024 03:29:25.2561
 (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: DC2I6FCBFt4UMof/WzbnPkcO/A4KaZ8MrBTpzlLPijiab8lKtDQ0qK+TviO2YZ1XVs1itZNR7e0roBvY72+UKg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR04MB8151

On 5/20/24 06:11, Dmitry Baryshkov wrote:
> On Thu, May 16, 2024 at 06:10:06PM +0800, Liu Ying wrote:
>> Commit f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
>> fails to consider the case where adv7511->i2c_main->irq is zero, i.e.,
>> no interrupt requested at all.
>>
>> Without interrupt, adv7511_wait_for_edid() could return -EIO sometimes,
>> because it polls adv7511->edid_read flag by calling adv7511_irq_process()
>> a few times, but adv7511_irq_process() happens to refuse to handle
>> interrupt by returning -ENODATA.  Hence, EDID retrieval fails randomly.
>>
>> Fix the issue by checking adv7511->i2c_main->irq before exiting interrupt
>> handling from adv7511_irq_process().
>>
>> Fixes: f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>> ---
>>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> index 6089b0bb9321..2074fa3c1b7b 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> @@ -479,7 +479,8 @@ static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd)
>>  		return ret;
>>  
>>  	/* If there is no IRQ to handle, exit indicating no IRQ data */
>> -	if (!(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
>> +	if (adv7511->i2c_main->irq &&
>> +	    !(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
>>  	    !(irq1 & ADV7511_INT1_DDC_ERROR))
>>  		return -ENODATA;
> 
> I think it might be better to handle -ENODATA in adv7511_wait_for_edid()
> instead. WDYT?

Then, adv7511_cec_irq_process() will have less chance to be called from
adv7511_irq_process() (assuming CONFIG_DRM_I2C_ADV7511_CEC is defined)
if adv7511->i2c_main->irq is zero.

But, anyway, it seems that commit f3d9683346d6 ("drm/bridge: adv7511:
Allow IRQ to share GPIO pins") is even more broken to handle the CEC case,
as adv7511_cec_adap_enable() may enable some interrupts for CEC.

This is a bit complicated.  Thoughts?

Regards,
Liu Ying