On Tue, Apr 1, 2014 at 5:45 PM, Daniel Vetter daniel@ffwll.ch wrote:
On Mon, Mar 31, 2014 at 06:03:24PM -0700, Matt Roper wrote:
On Fri, Mar 28, 2014 at 09:32:06AM +0100, Daniel Vetter wrote:
On Thu, Mar 27, 2014 at 05:44:31PM -0700, Matt Roper wrote:
...
- N.B., we call set_config() directly here rather than using
- drm_mode_set_config_internal. We're reprogramming the same
- connectors that were already in use, so we shouldn't need the extra
- cross-CRTC fb refcounting to accomodate stealing connectors.
- drm_mode_setplane() already handles the basic refcounting for the
- framebuffers involved in this operation.
Wrong. The current crtc helper logic disables all disconnected connectors forcefully on each set_config. Nope, I didn't make those semantics ... So I think we need to think a bit harder here again.
See drm_helper_disable_unused_functions.
I think I'm still missing something here; can you clarify what the problematic case would be?
I only see a call to __drm_helper_disable_unused_functions() in the CRTC helper in cases where mode_changed = 1, which I don't believe it ever should be through the setplane() calls. We should just be updating the framebuffer (and possibly panning) without touching any of the connectors, so I don't see how unrelated CRTC's would get disabled. Am I overlooking another case here that the basic refcounting in setplane doesn't already handle?
Looking at drm_helper_disable_unused_functions we'll spot
list_for_each_entry(connector, &dev->mode_config.connector_list, head) { if (!connector->encoder) continue; if (connector->status == connector_status_disconnected) connector->encoder = NULL; }So as soon as a connector is disconnected and you do _any_ kind of ->set_config call with modesetting helpers that crtc gets killed. Even if it's completely unrelated. Dave originally changed this with an imo rather thin justification:
commit a3a0544b2c84e1d7a2022b558ecf66d8c6a8dd93 Author: Dave Airlie airlied@redhat.com Date: Mon Aug 31 15:16:30 2009 +1000
drm/kms: add explicit encoder disable function and detach harder. For shared tv-out and VGA encoders, we really need to know if the encoder is just being switched off temporarily in blanking or if we are really disabling it hard. Also we need to try harder to disconnect encoders from unused connectors so we can share more efficently. (shared encoders stuff is coming in radeon tv-out support) Signed-off-by: Dave Airlie <airlied@redhat.com>To me this always smelled like papering over broken userspace. I've killed this in the i915 modeset rewrite and we didn't really have angry users scaling our walls. But I'm not sure what'll happen if we do this for all other drivers.
I've had to look at this again recently, and while I still don't like my commit, its not papering over userspace, it might be papering over fbcon :-)
You don't have any hw that operates like this, so I'd be surprised if you had users falling over,
The problem is if I have a single DAC encoder, with tv-out and VGA connectors, and I unplug the VGA connector, and plug in the tv connector, how do I get fbcon to pop-up,
Though that said this commit caused a regression that I'm not sure I liked either, since I think we used to allow you to force a mode on disconnected outputs, and this stops that from working, I noticed in a virtual env.
Dave.