| Message ID | 1431548627-2527-9-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, May 13, 2015 at 10:23:38PM +0200, Maarten Lankhorst wrote: > From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > > A follow up patch will make intel_modeset_compute_config() deal with > multiple crtcs, so move crtc specific stuff into the lower level crtc > specific function. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++++++++++++---------------- > 1 file changed, 28 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 583c9105cf49..ec548cbb06ee 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -427,6 +427,12 @@ static void vlv_clock(int refclk, intel_clock_t *clock) > clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p); > } > > +static bool > +needs_modeset(struct drm_crtc_state *state) > +{ > + return state->mode_changed || state->active_changed; > +} > + > /** > * Returns whether any output on the specified pipe is of the specified type > */ > @@ -11387,6 +11393,15 @@ intel_modeset_pipe_config(struct drm_crtc *crtc, > return -EINVAL; > } > > + /* > + * XXX: Add all connectors to make the crtc state match the encoders. > + */ > + if (!needs_modeset(&pipe_config->base)) { > + ret = drm_atomic_add_affected_connectors(state, crtc); > + if (ret) > + return ret; > + } Comment aside: Eventually we need to invert this check and use intel_pipe_config_compare with a special mode to a) not scream into dmesg b) ignore any changes we can fixup without a full modeset (i.e. pfit). And then use that to decide whether we need a modeset or not for the crtc. We still rely upon our internal set_mode functions to at least in some cases do an unconditional modeset on the passed-in crtc for property updates. But once converted to atomic we need to be more intelligent, especially since userspace expects no-op changes to get filtered out. So unconditionally doing a modeset wont cut it either (with the current code all the set_prop hooks have such checks hand-rolled). -Daniel > + > clear_intel_crtc_state(pipe_config); > > pipe_config->cpu_transcoder = > @@ -11478,6 +11493,18 @@ encoder_retry: > DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n", > base_bpp, pipe_config->pipe_bpp, pipe_config->dither); > > + /* Check if we need to force a modeset */ > + if (pipe_config->has_audio != > + to_intel_crtc_state(crtc->state)->has_audio) > + pipe_config->base.mode_changed = true; > + > + /* > + * Note we have an issue here with infoframes: current code > + * only updates them on the full mode set path per hw > + * requirements. So here we should be checking for any > + * required changes and forcing a mode set. > + */ > + > return 0; > fail: > return ret; > @@ -11495,12 +11522,6 @@ static bool intel_crtc_in_use(struct drm_crtc *crtc) > return false; > } > > -static bool > -needs_modeset(struct drm_crtc_state *state) > -{ > - return state->mode_changed || state->active_changed; > -} > - > static void > intel_modeset_update_state(struct drm_atomic_state *state) > { > @@ -12093,10 +12114,6 @@ intel_modeset_compute_config(struct drm_crtc *crtc, > struct intel_crtc_state *pipe_config; > int ret = 0; > > - ret = drm_atomic_add_affected_connectors(state, crtc); > - if (ret) > - return ERR_PTR(ret); > - > ret = drm_atomic_helper_check_modeset(state->dev, state); > if (ret) > return ERR_PTR(ret); > @@ -12122,19 +12139,7 @@ intel_modeset_compute_config(struct drm_crtc *crtc, > if (ret) > return ERR_PTR(ret); > > - /* Check things that can only be changed through modeset */ > - if (pipe_config->has_audio != > - to_intel_crtc(crtc)->config->has_audio) > - pipe_config->base.mode_changed = true; > - > - /* > - * Note we have an issue here with infoframes: current code > - * only updates them on the full mode set path per hw > - * requirements. So here we should be checking for any > - * required changes and forcing a mode set. > - */ > - > - intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,"[modeset]"); > + intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config, "[modeset]"); > > ret = drm_atomic_helper_check_planes(state->dev, state); > if (ret) > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 18-05-15 om 17:36 schreef Daniel Vetter: > On Wed, May 13, 2015 at 10:23:38PM +0200, Maarten Lankhorst wrote: >> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> >> >> A follow up patch will make intel_modeset_compute_config() deal with >> multiple crtcs, so move crtc specific stuff into the lower level crtc >> specific function. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++++++++++++---------------- >> 1 file changed, 28 insertions(+), 23 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 583c9105cf49..ec548cbb06ee 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -427,6 +427,12 @@ static void vlv_clock(int refclk, intel_clock_t *clock) >> clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p); >> } >> >> +static bool >> +needs_modeset(struct drm_crtc_state *state) >> +{ >> + return state->mode_changed || state->active_changed; >> +} >> + >> /** >> * Returns whether any output on the specified pipe is of the specified type >> */ >> @@ -11387,6 +11393,15 @@ intel_modeset_pipe_config(struct drm_crtc *crtc, >> return -EINVAL; >> } >> >> + /* >> + * XXX: Add all connectors to make the crtc state match the encoders. >> + */ >> + if (!needs_modeset(&pipe_config->base)) { >> + ret = drm_atomic_add_affected_connectors(state, crtc); >> + if (ret) >> + return ret; >> + } > Comment aside: Eventually we need to invert this check and use > intel_pipe_config_compare with a special mode to a) not scream into dmesg > b) ignore any changes we can fixup without a full modeset (i.e. pfit). And > then use that to decide whether we need a modeset or not for the crtc. > We still rely upon our internal set_mode functions to at least in some > cases do an unconditional modeset on the passed-in crtc for property > updates. But once converted to atomic we need to be more intelligent, > especially since userspace expects no-op changes to get filtered out. So > unconditionally doing a modeset wont cut it either (with the current code > all the set_prop hooks have such checks hand-rolled). Yeah, but for now I can't get that to work properly until everything looks more atomic first. ~Maarten
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 583c9105cf49..ec548cbb06ee 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -427,6 +427,12 @@ static void vlv_clock(int refclk, intel_clock_t *clock) clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p); } +static bool +needs_modeset(struct drm_crtc_state *state) +{ + return state->mode_changed || state->active_changed; +} + /** * Returns whether any output on the specified pipe is of the specified type */ @@ -11387,6 +11393,15 @@ intel_modeset_pipe_config(struct drm_crtc *crtc, return -EINVAL; } + /* + * XXX: Add all connectors to make the crtc state match the encoders. + */ + if (!needs_modeset(&pipe_config->base)) { + ret = drm_atomic_add_affected_connectors(state, crtc); + if (ret) + return ret; + } + clear_intel_crtc_state(pipe_config); pipe_config->cpu_transcoder = @@ -11478,6 +11493,18 @@ encoder_retry: DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n", base_bpp, pipe_config->pipe_bpp, pipe_config->dither); + /* Check if we need to force a modeset */ + if (pipe_config->has_audio != + to_intel_crtc_state(crtc->state)->has_audio) + pipe_config->base.mode_changed = true; + + /* + * Note we have an issue here with infoframes: current code + * only updates them on the full mode set path per hw + * requirements. So here we should be checking for any + * required changes and forcing a mode set. + */ + return 0; fail: return ret; @@ -11495,12 +11522,6 @@ static bool intel_crtc_in_use(struct drm_crtc *crtc) return false; } -static bool -needs_modeset(struct drm_crtc_state *state) -{ - return state->mode_changed || state->active_changed; -} - static void intel_modeset_update_state(struct drm_atomic_state *state) { @@ -12093,10 +12114,6 @@ intel_modeset_compute_config(struct drm_crtc *crtc, struct intel_crtc_state *pipe_config; int ret = 0; - ret = drm_atomic_add_affected_connectors(state, crtc); - if (ret) - return ERR_PTR(ret); - ret = drm_atomic_helper_check_modeset(state->dev, state); if (ret) return ERR_PTR(ret); @@ -12122,19 +12139,7 @@ intel_modeset_compute_config(struct drm_crtc *crtc, if (ret) return ERR_PTR(ret); - /* Check things that can only be changed through modeset */ - if (pipe_config->has_audio != - to_intel_crtc(crtc)->config->has_audio) - pipe_config->base.mode_changed = true; - - /* - * Note we have an issue here with infoframes: current code - * only updates them on the full mode set path per hw - * requirements. So here we should be checking for any - * required changes and forcing a mode set. - */ - - intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,"[modeset]"); + intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config, "[modeset]"); ret = drm_atomic_helper_check_planes(state->dev, state); if (ret)