On Wed, Mar 04, 2015 at 04:33:17PM +0100, Daniel Vetter wrote:
> On Tue, Mar 03, 2015 at 03:21:59PM +0200, Ander Conselvan de Oliveira wrote:
> > For the atomic conversion, the mode set paths need to be changed to rely
> > on an atomic state instead of using the staged config. By using an
> > atomic state for the legacy code, we will be able to convert the code
> > base in small chunks.
> >
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx>
>
> Two small comments below.
> -Daniel
>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 118 +++++++++++++++++++++++++++--------
> > 1 file changed, 91 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 798de7b..97d4df5 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -37,6 +37,7 @@
> > #include <drm/i915_drm.h>
> > #include "i915_drv.h"
> > #include "i915_trace.h"
> > +#include <drm/drm_atomic.h>
> > #include <drm/drm_atomic_helper.h>
> > #include <drm/drm_dp_helper.h>
> > #include <drm/drm_crtc_helper.h>
> > @@ -10290,10 +10291,22 @@ static bool check_digital_port_conflicts(struct drm_device *dev)
> > return true;
> > }
> >
> > -static struct intel_crtc_state *
> > +static void
> > +clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> > +{
> > + struct drm_crtc_state tmp_state;
> > +
> > + /* Clear only the intel specific part of the crtc state */
> > + tmp_state = crtc_state->base;
> > + memset(crtc_state, 0, sizeof *crtc_state);
> > + crtc_state->base = tmp_state;
> > +}
>
> I guess this is to clear out state which we want to recompute, and our
> compute_config code assumes that it's always kzalloc'ed a new config?
>
> I think this should be part of the crtc duplicate_state callback to make
> sure we're doing this consistently.
If we zero out the config in duplicate_state(), then we're not really
"duplicating" anymore are we? Ultimately we want to be able to
duplicate the existing state, tweak a couple things, and then pass that
state through the atomic pipeline, so it feels like doing a clear in the
duplicate function is the wrong move, even if it would work for the
frankenstein-style codebase we have at the moment.
Matt
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
