Hi Tomi,
On Friday 02 Dec 2016 16:22:18 Tomi Valkeinen wrote:
On 02/12/16 16:16, Laurent Pinchart wrote:
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@ti.com
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 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.
I can understand that (even if I'm not sure it's really an issue, and we should really clean up the CRTC creation code at some point), but how about adding a possible_crtcs field to the priv structure then ? I don't really like having to pass it around through a bunch of functions.