Re: [PATCH v2 01/22] drm: add clone mode check for CRTC

[Jessica Zhang <quic_jesszhan@quicinc.com>]

On 9/25/2024 1:12 AM, Jani Nikula wrote:
> On Tue, 24 Sep 2024, Jessica Zhang <quic_jesszhan@quicinc.com> wrote:
>> Add helper to check if the given CRTC state is in clone mode
>>
>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>> ---
>>   include/drm/drm_crtc.h | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 8b48a1974da3..ecb93e2c4afc 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -1323,5 +1323,12 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
>>   
>>   int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
>>   					    unsigned int supported_filters);
>> +static inline bool drm_crtc_in_clone_mode(struct drm_crtc_state *crtc_state)
>> +{
>> +	if (!crtc_state)
>> +		return false;
>> +
>> +	return hweight32(crtc_state->encoder_mask) > 1;
>> +}
> 
> What's the benefit of this being static inline?
> 
> You're implicitly depending on hweight32() being available, basically
> <linux/bitops.h> being included. Maybe it already is, but it's the
> accumulation of small and innocent looking things like this that then
> explode the header dependencies, and make them harder to reduce.

Hi Jani,

Good point, I'll move the implementation to drm_crtc.c.

Thanks,

Jessica Zhang

> 
> BR,
> Jani.
> 
>>   
>>   #endif /* __DRM_CRTC_H__ */
> 
> -- 
> Jani Nikula, Intel