[PATCH v2 1/4] drm: rcar-du: dsi: Properly stop video mode TX

[Tomi Valkeinen]

Hi,

On 22/08/2022 16:25, Biju Das wrote:
> Hi Tomi,
> 
> Thanks for the patch.
> 
>> Subject: [PATCH v2 1/4] drm: rcar-du: dsi: Properly stop video mode TX
>>
>> From: Tomi Valkeinen <tomi.valkeinen+renesas at ideasonboard.com>
>>
>> The driver does not explicitly stop the video mode transmission when
>> disabling the output. While this doesn't seem to be causing any issues,
>> lets follow the steps described in the documentation and add a
>> rcar_mipi_dsi_stop_video() which stop the video mode transmission. This
>> function will also be used in later patches to stop the video
>> transmission even if the DSI IP is not shut down.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas at ideasonboard.com>
>> ---
>>   drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 29 +++++++++++++++++++++++++
>>   1 file changed, 29 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
>> b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
>> index 62f7eb84ab01..7f2be490fcf8 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
>> @@ -542,6 +542,34 @@ static int rcar_mipi_dsi_start_video(struct
>> rcar_mipi_dsi *dsi)
>>   	return 0;
>>   }
>>
>> +static void rcar_mipi_dsi_stop_video(struct rcar_mipi_dsi *dsi) {
>> +	u32 status;
>> +	int ret;
>> +
>> +	/* Disable transmission in video mode. */
>> +	rcar_mipi_dsi_clr(dsi, TXVMCR, TXVMCR_EN_VIDEO);
>> +
>> +	ret = read_poll_timeout(rcar_mipi_dsi_read, status,
>> +				!(status & TXVMSR_ACT),
>> +				2000, 100000, false, dsi, TXVMSR);
>> +	if (ret < 0) {
>> +		dev_err(dsi->dev, "Failed to disable video transmission\n");
>> +		return;
>> +	}
>> +
>> +	/* Assert video FIFO clear. */
>> +	rcar_mipi_dsi_set(dsi, TXVMCR, TXVMCR_VFCLR);
>> +
>> +	ret = read_poll_timeout(rcar_mipi_dsi_read, status,
>> +				!(status & TXVMSR_VFRDY),
>> +				2000, 100000, false, dsi, TXVMSR);
>> +	if (ret < 0) {
>> +		dev_err(dsi->dev, "Failed to assert video FIFO clear\n");
>> +		return;
> 
> This return is not required.

That is true, but I'd personally rather keep it there. If the return is 
removed, I can imagine how easily one could add a new piece of code in 
the end of the function, not realizing that one also needs to add a 
return (the one above) so that the code works correctly.

It just feels a bit safer this way.

  Tomi