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

[Laurent Pinchart]

Hi Peter,

On Monday, 4 September 2017 12:19:19 EEST Peter Ujfalusi wrote:
> 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.

Making it explicit that the variable can't be negative allows the compiler to 
warn when encoutering a < 0 test. It also makes the code self-documented, 
showing the reader that the variable isn't expected to become negative.

> >>  	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.

-- 
Regards,

Laurent Pinchart