Re: [PATCH v6 10/12] drm/bridge: cdns-dsi: Move DSI mode check to _atomic_check()

[Aradhya Bhatia]

On 1/13/25 14:43, Dmitry Baryshkov wrote:
> On Sun, Jan 12, 2025 at 12:57:36AM +0530, Aradhya Bhatia wrote:
>> From: Aradhya Bhatia <a-bhatia1@xxxxxx>
>>
>> At present, the DSI mode configuration check happens during the
>> _atomic_enable() phase, which is not really the best place for this.
>> Moreover, if the mode is not valid, the driver gives a warning and
>> continues the hardware configuration.
>>
>> Move the DSI mode configuration check to _atomic_check() instead, which
>> can properly report back any invalid mode, before the _enable phase even
>> begins.
>>
>> Signed-off-by: Aradhya Bhatia <a-bhatia1@xxxxxx>
>> Signed-off-by: Aradhya Bhatia <aradhya.bhatia@xxxxxxxxx>
>> ---
>>  .../gpu/drm/bridge/cadence/cdns-dsi-core.c    | 87 +++++++++++++++++--
>>  .../gpu/drm/bridge/cadence/cdns-dsi-core.h    |  1 +
>>  2 files changed, 83 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.h b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.h
>> index 5db5dbbbcaad..b785df45bc59 100644
>> --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.h
>> +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.h
>> @@ -77,6 +77,7 @@ struct cdns_dsi {
>>  	bool link_initialized;
>>  	bool phy_initialized;
>>  	struct phy *dphy;
>> +	struct cdns_dsi_cfg dsi_cfg;
> 
> Is this still something necessary / useful? I think the point was to
> move dsi_cfg to the state, while this is a non-state struct.

No, this isn't necessary. This is a stray piece of code. Looks like I
missed it. Thank you! I will drop this in the next revision.

Regards
Aradhya

> 
>>  };
>>  
>>  #endif /* !__CDNS_DSI_H__ */
>> -- 
>> 2.34.1
>>
>