Re: [RFC 3/7] drm/omap: Manage the usable omap_dss_device list within omap_drm_private

[Peter Ujfalusi <peter.ujfalusi@ti.com>]

Hi Laurent,


Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

On 2017-09-01 14:27, Laurent Pinchart wrote:
>> -static void omap_disconnect_dssdevs(void)
>> +static void omap_disconnect_dssdevs(struct drm_device *ddev)
>>  {
>> -	struct omap_dss_device *dssdev = NULL;
>> +	struct omap_drm_private *priv = ddev->dev_private;
>> +	int i;
>> +
>> +	for (i = 0; i < priv->num_dssdevs; i++) {
> 
> is is never negative, you can make it an unsigned int. This comment applies 
> through the whole patch.

True, are there any benefits to have unsigned int compared to int? I
don't think we are going to hit size limitation with the int, but if you
prefer to have unsigned int, then sure, I can change the num_displays
and 'i's to unsigned.

>>  	crtc_idx = 0;
>>  	plane_idx = 0;
>> -	for_each_dss_dev(dssdev) {
>> +	for (i = 0; i < priv->num_dssdevs; i++) {
>> +		struct omap_dss_device *dssdev = priv->dssdevs[i];
>>  		struct drm_connector *connector;
>>  		struct drm_encoder *encoder;
>>  		struct drm_plane *plane;
>>  		struct drm_crtc *crtc;
>>
>> -		if (!omapdss_device_is_connected(dssdev))
>> -			continue;
>> -
> 
> I believe this hunk is correct as dss devices are only disconnected by calls 
> to the oma_dss_driver .disconnect() operation, which is only called from 
> omap_disconnect_dssdevs(), but you should at the very least explain why in the 
> commit message.

The check became irrelevant as we did not add dssdevs to the array if
their connect failed and as you have said we only disconnect ddsdevs in
omap_disconnect_dssdevs() - in cleanup paths.

I will update the commit message with this information.

- Péter

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