[Intel-gfx] [PATCH 1/6] drm/i915: Use atomic state to obtain load detection crtc.
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Tue Feb 2 12:46:03 UTC 2016
Op 01-02-16 om 18:09 schreef Ville Syrjälä:
> On Mon, Feb 01, 2016 at 02:43:57PM +0100, Maarten Lankhorst wrote:
>> Instead of restoring dpms and a flag for whether a temp fb is allocated duplicate
>> the old plane_state and crtc_state, and restore the members we potentially touched.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 128 ++++++++++++++++++++---------------
>> drivers/gpu/drm/i915/intel_drv.h | 4 +-
>> 2 files changed, 76 insertions(+), 56 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 4d8c9f7857db..0702ce8ec36a 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -10361,6 +10361,7 @@ mode_fits_in_fbdev(struct drm_device *dev,
>> if (obj->base.size < mode->vdisplay * fb->pitches[0])
>> return NULL;
>>
>> + drm_framebuffer_reference(fb);
>> return fb;
>> #else
>> return NULL;
>> @@ -10426,6 +10427,9 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
>> encoder->base.id, encoder->name);
>>
>> retry:
>> + old->old_pipe_config = NULL;
>> + old->old_plane_state = NULL;
>> +
>> ret = drm_modeset_lock(&config->connection_mutex, ctx);
>> if (ret)
>> goto fail;
>> @@ -10441,24 +10445,15 @@ retry:
>> */
>>
>> /* See if we already have a CRTC for this connector */
>> - if (encoder->crtc) {
>> - crtc = encoder->crtc;
>> + if (connector->state->crtc) {
> All these connector->state accesses made me a bit uneasy, but we did
> indeed grab connection_mutex already so it should be fine. It's even
> more troubling seeing connector->state accessed outside
> intel_get_load_detect_pipe() in the later patches, but it seems it's
> only done if intel_get_load_detect_pipe() succeeded which means we
> should be holding the right lock.
>
> Dunno, maybe there should be some comments explaining this stuff.
> Or maybe maybe we should have a helper to return the current
> connector state that also asserts that the right lock is held?
I'm not sure how to proceed on that, I do think all accesses should be cleaned up,
but I'm not sure yet what the path forward is, and so I want to leave it for a different series.
More information about the Intel-gfx
mailing list