Hi Laurent, > -----Original Message----- > From: Laurent Pinchart [mailto:laurent.pinchart+renesas@xxxxxxxxxxxxxxxx] > Sent: Monday, March 02, 2015 5:20 AM > To: dri-devel@xxxxxxxxxxxxxxxxxxxxx > Cc: Lars-Peter Clausen; Chris Kohn; Hyun Kwon > Subject: [PATCH] drm: adv7511: Refactor power management > > From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > Remove the internal dependency on DPMS mode for power management by > using a by a powered state boolean instead, and use the new power off handler > at probe time. This ensure that the regmap cache is properly marked as dirty > when the device is probed, and the registers properly synced during the first > power up. > > As a side effect this removes the initialization of current_edid_segment at probe > time, as the field will be initialized when the device is powered on, at the latest > right before reading EDID data. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> Works fine on my platform. Tested-by: Christian Kohn <christian.kohn@xxxxxxxxxx> Cheers, Chris > --- > drivers/gpu/drm/i2c/adv7511.c | 97 +++++++++++++++++++++++------------------ > -- > 1 file changed, 51 insertions(+), 46 deletions(-) > > diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511.c > index fa140e04d5fa..7fb7e22f4ad1 100644 > --- a/drivers/gpu/drm/i2c/adv7511.c > +++ b/drivers/gpu/drm/i2c/adv7511.c > @@ -27,7 +27,7 @@ struct adv7511 { > struct regmap *regmap; > struct regmap *packet_memory_regmap; > enum drm_connector_status status; > - int dpms_mode; > + bool powered; > > unsigned int f_tmds; > > @@ -357,6 +357,46 @@ static void adv7511_set_link_config(struct adv7511 > *adv7511, > adv7511->rgb = config->input_colorspace == HDMI_COLORSPACE_RGB; > } > > +static void adv7511_power_on(struct adv7511 *adv7511) { > + adv7511->current_edid_segment = -1; > + > + regmap_write(adv7511->regmap, ADV7511_REG_INT(0), > + ADV7511_INT0_EDID_READY | > ADV7511_INT1_DDC_ERROR); > + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, > + ADV7511_POWER_POWER_DOWN, 0); > + > + /* > + * Per spec it is allowed to pulse the HDP signal to indicate that the > + * EDID information has changed. Some monitors do this when they > wakeup > + * from standby or are enabled. When the HDP goes low the adv7511 is > + * reset and the outputs are disabled which might cause the monitor to > + * go to standby again. To avoid this we ignore the HDP pin for the > + * first few seconds after enabling the output. > + */ > + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2, > + ADV7511_REG_POWER2_HDP_SRC_MASK, > + ADV7511_REG_POWER2_HDP_SRC_NONE); > + > + /* > + * Most of the registers are reset during power down or when HPD is > low. > + */ > + regcache_sync(adv7511->regmap); > + > + adv7511->powered = true; > +} > + > +static void adv7511_power_off(struct adv7511 *adv7511) { > + /* TODO: setup additional power down modes */ > + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, > + ADV7511_POWER_POWER_DOWN, > + ADV7511_POWER_POWER_DOWN); > + regcache_mark_dirty(adv7511->regmap); > + > + adv7511->powered = false; > +} > + > /* ----------------------------------------------------------------------------- > * Interrupt and hotplug detection > */ > @@ -526,7 +566,7 @@ static int adv7511_get_modes(struct drm_encoder > *encoder, > unsigned int count; > > /* Reading the EDID only works if the device is powered */ > - if (adv7511->dpms_mode != DRM_MODE_DPMS_ON) { > + if (!adv7511->powered) { > regmap_write(adv7511->regmap, ADV7511_REG_INT(0), > ADV7511_INT0_EDID_READY | > ADV7511_INT1_DDC_ERROR); > regmap_update_bits(adv7511->regmap, > ADV7511_REG_POWER, @@ -536,7 +576,7 @@ static int > adv7511_get_modes(struct drm_encoder *encoder, > > edid = drm_do_get_edid(connector, adv7511_get_edid_block, > adv7511); > > - if (adv7511->dpms_mode != DRM_MODE_DPMS_ON) > + if (!adv7511->powered) > regmap_update_bits(adv7511->regmap, > ADV7511_REG_POWER, > ADV7511_POWER_POWER_DOWN, > ADV7511_POWER_POWER_DOWN); > @@ -558,41 +598,10 @@ static void adv7511_encoder_dpms(struct > drm_encoder *encoder, int mode) { > struct adv7511 *adv7511 = encoder_to_adv7511(encoder); > > - switch (mode) { > - case DRM_MODE_DPMS_ON: > - adv7511->current_edid_segment = -1; > - > - regmap_write(adv7511->regmap, ADV7511_REG_INT(0), > - ADV7511_INT0_EDID_READY | > ADV7511_INT1_DDC_ERROR); > - regmap_update_bits(adv7511->regmap, > ADV7511_REG_POWER, > - ADV7511_POWER_POWER_DOWN, 0); > - /* > - * Per spec it is allowed to pulse the HDP signal to indicate > - * that the EDID information has changed. Some monitors do > this > - * when they wakeup from standby or are enabled. When the > HDP > - * goes low the adv7511 is reset and the outputs are disabled > - * which might cause the monitor to go to standby again. To > - * avoid this we ignore the HDP pin for the first few seconds > - * after enabeling the output. > - */ > - regmap_update_bits(adv7511->regmap, > ADV7511_REG_POWER2, > - ADV7511_REG_POWER2_HDP_SRC_MASK, > - ADV7511_REG_POWER2_HDP_SRC_NONE); > - /* Most of the registers are reset during power down or > - * when HPD is low > - */ > - regcache_sync(adv7511->regmap); > - break; > - default: > - /* TODO: setup additional power down modes */ > - regmap_update_bits(adv7511->regmap, > ADV7511_REG_POWER, > - ADV7511_POWER_POWER_DOWN, > - ADV7511_POWER_POWER_DOWN); > - regcache_mark_dirty(adv7511->regmap); > - break; > - } > - > - adv7511->dpms_mode = mode; > + if (mode == DRM_MODE_DPMS_ON) > + adv7511_power_on(adv7511); > + else > + adv7511_power_off(adv7511); > } > > static enum drm_connector_status > @@ -620,10 +629,9 @@ adv7511_encoder_detect(struct drm_encoder > *encoder, > * there is a pending HPD interrupt and the cable is connected there was > * at least one transition from disconnected to connected and the chip > * has to be reinitialized. */ > - if (status == connector_status_connected && hpd && > - adv7511->dpms_mode == DRM_MODE_DPMS_ON) { > + if (status == connector_status_connected && hpd && adv7511- > >powered) { > regcache_mark_dirty(adv7511->regmap); > - adv7511_encoder_dpms(encoder, adv7511->dpms_mode); > + adv7511_power_on(adv7511); > adv7511_get_modes(encoder, connector); > if (adv7511->status == connector_status_connected) > status = connector_status_disconnected; @@ -858,7 > +866,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct > i2c_device_id *id) > if (!adv7511) > return -ENOMEM; > > - adv7511->dpms_mode = DRM_MODE_DPMS_OFF; > + adv7511->powered = false; > adv7511->status = connector_status_disconnected; > > ret = adv7511_parse_dt(dev->of_node, &link_config); @@ -918,10 > +926,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct > i2c_device_id *id) > regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, > ADV7511_CEC_CTRL_POWER_DOWN); > > - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, > - ADV7511_POWER_POWER_DOWN, > ADV7511_POWER_POWER_DOWN); > - > - adv7511->current_edid_segment = -1; > + adv7511_power_off(adv7511); > > i2c_set_clientdata(i2c, adv7511); > > -- > Regards, > > Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
