On 02/12/16 16:16, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Friday 02 Dec 2016 16:07:11 Tomi Valkeinen wrote: >> We set the possible_crtc for all planes to "(1 << priv->num_crtcs) - 1", >> which is fine as the HW planes can be used fro all crtcs. However, when >> we're doing that, we are still incrementing 'num_crtcs', and we'll end >> up with bad possible_crtcs, preventing the use of the primary planes. >> >> This patch passes a possible_crtcs mask to plane init function so that >> we get correct possible_crtc. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx> >> --- >> >> v2: use correct possible_crtcs value >> >> drivers/gpu/drm/omapdrm/omap_drv.c | 17 ++++++++++++----- >> drivers/gpu/drm/omapdrm/omap_drv.h | 3 ++- >> drivers/gpu/drm/omapdrm/omap_plane.c | 6 +++--- >> 3 files changed, 17 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c >> b/drivers/gpu/drm/omapdrm/omap_drv.c index 39c5312b466c..fdc83cbcde61 >> 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c >> @@ -267,13 +267,15 @@ static int omap_connect_dssdevs(void) >> } >> >> static int omap_modeset_create_crtc(struct drm_device *dev, int id, >> - enum omap_channel channel) >> + enum omap_channel channel, >> + u32 possible_crtcs) >> { >> struct omap_drm_private *priv = dev->dev_private; >> struct drm_plane *plane; >> struct drm_crtc *crtc; >> >> - plane = omap_plane_init(dev, id, DRM_PLANE_TYPE_PRIMARY); >> + plane = omap_plane_init(dev, id, DRM_PLANE_TYPE_PRIMARY, >> + possible_crtcs); >> if (IS_ERR(plane)) >> return PTR_ERR(plane); > > If you removed the priv->num_crtcs++ a bit below in this function... > >> @@ -309,6 +311,7 @@ static int omap_modeset_init(struct drm_device *dev) >> int num_crtcs; >> int i, id = 0; >> int ret; >> + u32 possible_crtcs; >> >> drm_mode_config_init(dev); >> >> @@ -325,6 +328,7 @@ static int omap_modeset_init(struct drm_device *dev) >> * We use the num_crtc argument to limit the number of crtcs we > create. >> */ >> num_crtcs = min3(num_crtc, num_mgrs, num_ovls); > > and assigned priv->num_crtcs here and replaced the channel_used() function > with a simple bitmask private to omap_modeset_init() you would end up with a > much simpler implementation that wouldn't require passing possible_crtcs > through a bunch of functions. Yes, I almost did that. But priv-num_crtcs tells the amount of crtcs in priv->crtcs. If we set priv->num_crtcs before actually creating those crtcs, I fear we could easily create a bug. The crtc+plane creation code is not the shortest one, so even if we wouldn't have a bug right now, I imagine it could be easy to write a helper func or such later, which uses priv->num_crtcs, and which gets called at some point when creating crtcs and planes... So I went the safe way. Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel