Re: [PATCH 8/9] drm/atomic: Remove drm_atomic_connectors_for_crtc.

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 07-12-15 om 11:34 schreef Thierry Reding:
> On Tue, Nov 24, 2015 at 10:34:35AM +0100, Maarten Lankhorst wrote:
> [...]
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index cee31d43cd1c..fb79c54b2aed 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -420,8 +420,6 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>>  	 * crtc only changed its mode but has the same set of connectors.
>>  	 */
>>  	for_each_crtc_in_state(state, crtc, crtc_state, i) {
>> -		int num_connectors;
>> -
>>  		/*
>>  		 * We must set ->active_changed after walking connectors for
>>  		 * otherwise an update that only changes active would result in
>> @@ -449,10 +447,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>>  		if (ret != 0)
>>  			return ret;
>>  
>> -		num_connectors = drm_atomic_connectors_for_crtc(state,
>> -								crtc);
>> -
>> -		if (crtc_state->enable != !!num_connectors) {
>> +		if (crtc_state->enable != !!crtc_state->connector_mask) {
> I have difficulty to doubly negate in my head, so something like this
> would be a lot clearer in my opinion:
Maybe if enable == !connector_mask?

> 	bool enable = crtc_state->connector_mask != 0;
>
> 	...
>
> 	if (crtc_state->enable != enable)
> 		...
>
> Or perhaps even:
>
> 	bool disable = crtc_state->connector_mask == 0;
>
> 	...
>
> 	if (crtc_state->enable == disable)
> 		...
>
>>  			DRM_DEBUG_ATOMIC("[CRTC:%d] enabled/connectors mismatch\n",
>>  					 crtc->base.id);
>>  
>> @@ -1668,7 +1663,7 @@ static int update_output_state(struct drm_atomic_state *state,
>>  		if (crtc == set->crtc)
>>  			continue;
>>  
>> -		if (!drm_atomic_connectors_for_crtc(state, crtc)) {
>> +		if (!crtc_state->connector_mask) {
> Similarly I think it would be more natural to say:
>
> 		if (crtc->state->connector_mask == 0)
>
> here.
>
> Anyway, this is mostly about personal taste, and the change looks
> correct to me (after checking twice that I got the double negation
> right), so:
>
> Reviewed-by: Thierry Reding <treding@nvidia.com>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel