Skip to content

Commit 9d9486e

Browse files
thomashvmwthomashvmw
committed
drm/vmwgfx: Fix up the implicit display unit handling
Make the connector is_implicit property immutable. As far as we know, no user-space application is writing to it. Also move the verification that all implicit display units scan out from the same framebuffer to atomic_check(). Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> Reviewed-by: Sinclair Yeh <syeh@vmware.com> Reviewed-by: Deepak Rawat <drawat@vmware.com>
1 parent 66502d4 commit 9d9486e

File tree

6 files changed

+99
-292
lines changed

6 files changed

+99
-292
lines changed

‎drivers/gpu/drm/vmwgfx/vmwgfx_drv.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -484,8 +484,6 @@ struct vmw_private {
484484
struct vmw_overlay *overlay_priv;
485485
struct drm_property *hotplug_mode_update_property;
486486
struct drm_property *implicit_placement_property;
487-
unsigned num_implicit;
488-
struct vmw_framebuffer *implicit_fb;
489487
struct mutex global_kms_state_mutex;
490488
spinlock_t cursor_lock;
491489
struct drm_atomic_state *suspend_state;

‎drivers/gpu/drm/vmwgfx/vmwgfx_kms.c

Lines changed: 88 additions & 189 deletions
Original file line numberDiff line numberDiff line change
@@ -457,21 +457,8 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
457457
struct drm_crtc *crtc = state->crtc;
458458
struct vmw_connector_state *vcs;
459459
struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
460-
struct vmw_private *dev_priv = vmw_priv(crtc->dev);
461-
struct vmw_framebuffer *vfb = vmw_framebuffer_to_vfb(new_fb);
462460

463461
vcs = vmw_connector_state_to_vcs(du->connector.state);
464-
465-
/* Only one active implicit framebuffer at a time. */
466-
mutex_lock(&dev_priv->global_kms_state_mutex);
467-
if (vcs->is_implicit && dev_priv->implicit_fb &&
468-
!(dev_priv->num_implicit == 1 && du->active_implicit)
469-
&& dev_priv->implicit_fb != vfb) {
470-
DRM_ERROR("Multiple implicit framebuffers "
471-
"not supported.\n");
472-
ret = -EINVAL;
473-
}
474-
mutex_unlock(&dev_priv->global_kms_state_mutex);
475462
}
476463

477464

@@ -1519,6 +1506,88 @@ static int vmw_kms_check_display_memory(struct drm_device *dev,
15191506
return 0;
15201507
}
15211508

1509+
/**
1510+
* vmw_crtc_state_and_lock - Return new or current crtc state with locked
1511+
* crtc mutex
1512+
* @state: The atomic state pointer containing the new atomic state
1513+
* @crtc: The crtc
1514+
*
1515+
* This function returns the new crtc state if it's part of the state update.
1516+
* Otherwise returns the current crtc state. It also makes sure that the
1517+
* crtc mutex is locked.
1518+
*
1519+
* Returns: A valid crtc state pointer or NULL. It may also return a
1520+
* pointer error, in particular -EDEADLK if locking needs to be rerun.
1521+
*/
1522+
static struct drm_crtc_state *
1523+
vmw_crtc_state_and_lock(struct drm_atomic_state *state, struct drm_crtc *crtc)
1524+
{
1525+
struct drm_crtc_state *crtc_state;
1526+
1527+
crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
1528+
if (crtc_state) {
1529+
lockdep_assert_held(&crtc->mutex.mutex.base);
1530+
} else {
1531+
int ret = drm_modeset_lock(&crtc->mutex, state->acquire_ctx);
1532+
1533+
if (ret != 0 && ret != -EALREADY)
1534+
return ERR_PTR(ret);
1535+
1536+
crtc_state = crtc->state;
1537+
}
1538+
1539+
return crtc_state;
1540+
}
1541+
1542+
/**
1543+
* vmw_kms_check_implicit - Verify that all implicit display units scan out
1544+
* from the same fb after the new state is committed.
1545+
* @dev: The drm_device.
1546+
* @state: The new state to be checked.
1547+
*
1548+
* Returns:
1549+
* Zero on success,
1550+
* -EINVAL on invalid state,
1551+
* -EDEADLK if modeset locking needs to be rerun.
1552+
*/
1553+
static int vmw_kms_check_implicit(struct drm_device *dev,
1554+
struct drm_atomic_state *state)
1555+
{
1556+
struct drm_framebuffer *implicit_fb = NULL;
1557+
struct drm_crtc *crtc;
1558+
struct drm_crtc_state *crtc_state;
1559+
struct drm_plane_state *plane_state;
1560+
1561+
drm_for_each_crtc(crtc, dev) {
1562+
struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
1563+
1564+
if (!du->is_implicit)
1565+
continue;
1566+
1567+
crtc_state = vmw_crtc_state_and_lock(state, crtc);
1568+
if (IS_ERR(crtc_state))
1569+
return PTR_ERR(crtc_state);
1570+
1571+
if (!crtc_state || !crtc_state->enable)
1572+
continue;
1573+
1574+
/*
1575+
* Can't move primary planes across crtcs, so this is OK.
1576+
* It also means we don't need to take the plane mutex.
1577+
*/
1578+
plane_state = du->primary.state;
1579+
if (plane_state->crtc != crtc)
1580+
continue;
1581+
1582+
if (!implicit_fb)
1583+
implicit_fb = plane_state->fb;
1584+
else if (implicit_fb != plane_state->fb)
1585+
return -EINVAL;
1586+
}
1587+
1588+
return 0;
1589+
}
1590+
15221591
/**
15231592
* vmw_kms_check_topology - Validates topology in drm_atomic_state
15241593
* @dev: DRM device
@@ -1636,6 +1705,10 @@ vmw_kms_atomic_check_modeset(struct drm_device *dev,
16361705
if (ret)
16371706
return ret;
16381707

1708+
ret = vmw_kms_check_implicit(dev, state);
1709+
if (ret)
1710+
return ret;
1711+
16391712
if (!state->allow_modeset)
16401713
return ret;
16411714

@@ -2230,84 +2303,6 @@ int vmw_du_connector_fill_modes(struct drm_connector *connector,
22302303
return 1;
22312304
}
22322305

2233-
int vmw_du_connector_set_property(struct drm_connector *connector,
2234-
struct drm_property *property,
2235-
uint64_t val)
2236-
{
2237-
struct vmw_display_unit *du = vmw_connector_to_du(connector);
2238-
struct vmw_private *dev_priv = vmw_priv(connector->dev);
2239-
2240-
if (property == dev_priv->implicit_placement_property)
2241-
du->is_implicit = val;
2242-
2243-
return 0;
2244-
}
2245-
2246-
2247-
2248-
/**
2249-
* vmw_du_connector_atomic_set_property - Atomic version of get property
2250-
*
2251-
* @crtc - crtc the property is associated with
2252-
*
2253-
* Returns:
2254-
* Zero on success, negative errno on failure.
2255-
*/
2256-
int
2257-
vmw_du_connector_atomic_set_property(struct drm_connector *connector,
2258-
struct drm_connector_state *state,
2259-
struct drm_property *property,
2260-
uint64_t val)
2261-
{
2262-
struct vmw_private *dev_priv = vmw_priv(connector->dev);
2263-
struct vmw_connector_state *vcs = vmw_connector_state_to_vcs(state);
2264-
struct vmw_display_unit *du = vmw_connector_to_du(connector);
2265-
2266-
2267-
if (property == dev_priv->implicit_placement_property) {
2268-
vcs->is_implicit = val;
2269-
2270-
/*
2271-
* We should really be doing a drm_atomic_commit() to
2272-
* commit the new state, but since this doesn't cause
2273-
* an immedate state change, this is probably ok
2274-
*/
2275-
du->is_implicit = vcs->is_implicit;
2276-
} else {
2277-
return -EINVAL;
2278-
}
2279-
2280-
return 0;
2281-
}
2282-
2283-
2284-
/**
2285-
* vmw_du_connector_atomic_get_property - Atomic version of get property
2286-
*
2287-
* @connector - connector the property is associated with
2288-
*
2289-
* Returns:
2290-
* Zero on success, negative errno on failure.
2291-
*/
2292-
int
2293-
vmw_du_connector_atomic_get_property(struct drm_connector *connector,
2294-
const struct drm_connector_state *state,
2295-
struct drm_property *property,
2296-
uint64_t *val)
2297-
{
2298-
struct vmw_private *dev_priv = vmw_priv(connector->dev);
2299-
struct vmw_connector_state *vcs = vmw_connector_state_to_vcs(state);
2300-
2301-
if (property == dev_priv->implicit_placement_property)
2302-
*val = vcs->is_implicit;
2303-
else {
2304-
DRM_ERROR("Invalid Property %s\n", property->name);
2305-
return -EINVAL;
2306-
}
2307-
2308-
return 0;
2309-
}
2310-
23112306
/**
23122307
* vmw_kms_update_layout_ioctl - Handler for DRM_VMW_UPDATE_LAYOUT ioctl
23132308
* @dev: drm device for the ioctl
@@ -2696,120 +2691,24 @@ int vmw_kms_fbdev_init_data(struct vmw_private *dev_priv,
26962691
return ret;
26972692
}
26982693

2699-
/**
2700-
* vmw_kms_del_active - unregister a crtc binding to the implicit framebuffer
2701-
*
2702-
* @dev_priv: Pointer to a device private struct.
2703-
* @du: The display unit of the crtc.
2704-
*/
2705-
void vmw_kms_del_active(struct vmw_private *dev_priv,
2706-
struct vmw_display_unit *du)
2707-
{
2708-
mutex_lock(&dev_priv->global_kms_state_mutex);
2709-
if (du->active_implicit) {
2710-
if (--(dev_priv->num_implicit) == 0)
2711-
dev_priv->implicit_fb = NULL;
2712-
du->active_implicit = false;
2713-
}
2714-
mutex_unlock(&dev_priv->global_kms_state_mutex);
2715-
}
2716-
2717-
/**
2718-
* vmw_kms_add_active - register a crtc binding to an implicit framebuffer
2719-
*
2720-
* @vmw_priv: Pointer to a device private struct.
2721-
* @du: The display unit of the crtc.
2722-
* @vfb: The implicit framebuffer
2723-
*
2724-
* Registers a binding to an implicit framebuffer.
2725-
*/
2726-
void vmw_kms_add_active(struct vmw_private *dev_priv,
2727-
struct vmw_display_unit *du,
2728-
struct vmw_framebuffer *vfb)
2729-
{
2730-
mutex_lock(&dev_priv->global_kms_state_mutex);
2731-
WARN_ON_ONCE(!dev_priv->num_implicit && dev_priv->implicit_fb);
2732-
2733-
if (!du->active_implicit && du->is_implicit) {
2734-
dev_priv->implicit_fb = vfb;
2735-
du->active_implicit = true;
2736-
dev_priv->num_implicit++;
2737-
}
2738-
mutex_unlock(&dev_priv->global_kms_state_mutex);
2739-
}
2740-
2741-
/**
2742-
* vmw_kms_screen_object_flippable - Check whether we can page-flip a crtc.
2743-
*
2744-
* @dev_priv: Pointer to device-private struct.
2745-
* @crtc: The crtc we want to flip.
2746-
*
2747-
* Returns true or false depending whether it's OK to flip this crtc
2748-
* based on the criterion that we must not have more than one implicit
2749-
* frame-buffer at any one time.
2750-
*/
2751-
bool vmw_kms_crtc_flippable(struct vmw_private *dev_priv,
2752-
struct drm_crtc *crtc)
2753-
{
2754-
struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
2755-
bool ret;
2756-
2757-
mutex_lock(&dev_priv->global_kms_state_mutex);
2758-
ret = !du->is_implicit || dev_priv->num_implicit == 1;
2759-
mutex_unlock(&dev_priv->global_kms_state_mutex);
2760-
2761-
return ret;
2762-
}
2763-
2764-
/**
2765-
* vmw_kms_update_implicit_fb - Update the implicit fb.
2766-
*
2767-
* @dev_priv: Pointer to device-private struct.
2768-
* @crtc: The crtc the new implicit frame-buffer is bound to.
2769-
*/
2770-
void vmw_kms_update_implicit_fb(struct vmw_private *dev_priv,
2771-
struct drm_crtc *crtc)
2772-
{
2773-
struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
2774-
struct drm_plane *plane = crtc->primary;
2775-
struct vmw_framebuffer *vfb;
2776-
2777-
mutex_lock(&dev_priv->global_kms_state_mutex);
2778-
2779-
if (!du->is_implicit)
2780-
goto out_unlock;
2781-
2782-
vfb = vmw_framebuffer_to_vfb(plane->state->fb);
2783-
WARN_ON_ONCE(dev_priv->num_implicit != 1 &&
2784-
dev_priv->implicit_fb != vfb);
2785-
2786-
dev_priv->implicit_fb = vfb;
2787-
out_unlock:
2788-
mutex_unlock(&dev_priv->global_kms_state_mutex);
2789-
}
2790-
27912694
/**
27922695
* vmw_kms_create_implicit_placement_proparty - Set up the implicit placement
27932696
* property.
27942697
*
27952698
* @dev_priv: Pointer to a device private struct.
2796-
* @immutable: Whether the property is immutable.
27972699
*
27982700
* Sets up the implicit placement property unless it's already set up.
27992701
*/
28002702
void
2801-
vmw_kms_create_implicit_placement_property(struct vmw_private *dev_priv,
2802-
bool immutable)
2703+
vmw_kms_create_implicit_placement_property(struct vmw_private *dev_priv)
28032704
{
28042705
if (dev_priv->implicit_placement_property)
28052706
return;
28062707

28072708
dev_priv->implicit_placement_property =
28082709
drm_property_create_range(dev_priv->dev,
2809-
immutable ?
2810-
DRM_MODE_PROP_IMMUTABLE : 0,
2710+
DRM_MODE_PROP_IMMUTABLE,
28112711
"implicit_placement", 0, 1);
2812-
28132712
}
28142713

28152714
/**

‎drivers/gpu/drm/vmwgfx/vmwgfx_kms.h

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,6 @@ struct vmw_plane_state {
307307
struct vmw_connector_state {
308308
struct drm_connector_state base;
309309

310-
bool is_implicit;
311-
312310
/**
313311
* @gui_x:
314312
*
@@ -370,7 +368,6 @@ struct vmw_display_unit {
370368
int gui_x;
371369
int gui_y;
372370
bool is_implicit;
373-
bool active_implicit;
374371
int set_gui_x;
375372
int set_gui_y;
376373
};
@@ -450,17 +447,8 @@ int vmw_kms_fbdev_init_data(struct vmw_private *dev_priv,
450447
struct drm_crtc **p_crtc,
451448
struct drm_display_mode **p_mode);
452449
void vmw_guess_mode_timing(struct drm_display_mode *mode);
453-
void vmw_kms_del_active(struct vmw_private *dev_priv,
454-
struct vmw_display_unit *du);
455-
void vmw_kms_add_active(struct vmw_private *dev_priv,
456-
struct vmw_display_unit *du,
457-
struct vmw_framebuffer *vfb);
458-
bool vmw_kms_crtc_flippable(struct vmw_private *dev_priv,
459-
struct drm_crtc *crtc);
460-
void vmw_kms_update_implicit_fb(struct vmw_private *dev_priv,
461-
struct drm_crtc *crtc);
462-
void vmw_kms_create_implicit_placement_property(struct vmw_private *dev_priv,
463-
bool immutable);
450+
void vmw_kms_update_implicit_fb(struct vmw_private *dev_priv);
451+
void vmw_kms_create_implicit_placement_property(struct vmw_private *dev_priv);
464452

465453
/* Universal Plane Helpers */
466454
void vmw_du_primary_plane_destroy(struct drm_plane *plane);

0 commit comments

Comments
 (0)