[PATCH 1/5] drm/bridge: tc358762: Split register programming from pre-enable to enable

[Dmitry Baryshkov]

On 16/06/2023 22:02, Sam Ravnborg wrote:
> Hi Marek,
> 
> On Thu, Jun 15, 2023 at 10:18:58PM +0200, Marek Vasut wrote:
>> Move the register programming part, which actually enables the bridge and
>> makes it push data out of its DPI side, into the enable callback. The DSI
>> host like DSIM may not be able to transmit commands in pre_enable, moving
>> the register programming into enable assures it can transmit commands.
>>
>> Signed-off-by: Marek Vasut <marex at denx.de>
> 
> I was about to complain that the use of .enable is deprecated, but the
> following patch fixes this.
> So:
> Reviewed-by: Sam Ravnborg <sam at ravnborg.org>

Dear drm-misc / bridge maintainers. About two years ago nearly the same 
patch was rejected ([1]) on the grounds of some hosts (sunxi) being 
unable to send DCS commands in HS state. Later Dave created and landed 
patches that potentially enable all peripherals to send DSI commands 
from the pre_enable callback ([2]) and tc358762 picked up usage of these 
flags ([3]).

And then we land this patch, which contradicts all previous steps.
I think I fail to understand your actions.

[1] 
https://lore.kernel.org/linux-arm-msm/CAPY8ntBrhYAmsraDqJGuTrSL6VjGXBAMVoN7xweV7E4qZv+v3Q@mail.gmail.com/

[2] 
https://lore.kernel.org/r/20221205173328.1395350-5-dave.stevenson@raspberrypi.com

[3] 
https://lore.kernel.org/dri-devel/20230131141756.RFT.v2.1.I723a3761d57ea60c5dd754c144aed6c3b2ea6f5a@changeid/

> 
> 
>> ---
>> Cc: Andrzej Hajda <andrzej.hajda at intel.com>
>> Cc: Daniel Vetter <daniel at ffwll.ch>
>> Cc: David Airlie <airlied at gmail.com>
>> Cc: Jernej Skrabec <jernej.skrabec at gmail.com>
>> Cc: Jonas Karlman <jonas at kwiboo.se>
>> Cc: Laurent Pinchart <Laurent.pinchart at ideasonboard.com>
>> Cc: Neil Armstrong <neil.armstrong at linaro.org>
>> Cc: Robert Foss <rfoss at kernel.org>
>> Cc: dri-devel at lists.freedesktop.org
>> ---
>>   drivers/gpu/drm/bridge/tc358762.c | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358762.c b/drivers/gpu/drm/bridge/tc358762.c
>> index 5641395fd310e..df9703eacab1f 100644
>> --- a/drivers/gpu/drm/bridge/tc358762.c
>> +++ b/drivers/gpu/drm/bridge/tc358762.c
>> @@ -162,11 +162,17 @@ static void tc358762_pre_enable(struct drm_bridge *bridge)
>>   		usleep_range(5000, 10000);
>>   	}
>>   
>> +	ctx->pre_enabled = true;
>> +}
>> +
>> +static void tc358762_enable(struct drm_bridge *bridge)
>> +{
>> +	struct tc358762 *ctx = bridge_to_tc358762(bridge);
>> +	int ret;
>> +
>>   	ret = tc358762_init(ctx);
>>   	if (ret < 0)
>>   		dev_err(ctx->dev, "error initializing bridge (%d)\n", ret);
>> -
>> -	ctx->pre_enabled = true;
>>   }
>>   
>>   static int tc358762_attach(struct drm_bridge *bridge,
>> @@ -181,6 +187,7 @@ static int tc358762_attach(struct drm_bridge *bridge,
>>   static const struct drm_bridge_funcs tc358762_bridge_funcs = {
>>   	.post_disable = tc358762_post_disable,
>>   	.pre_enable = tc358762_pre_enable,
>> +	.enable = tc358762_enable,
>>   	.attach = tc358762_attach,
>>   };
>>   
>> -- 
>> 2.39.2

-- 
With best wishes
Dmitry