All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: John Stultz <john.stultz@linaro.org>
Cc: lkml <linux-kernel@vger.kernel.org>,
	dri-devel@lists.freedesktop.org,
	Wolfram Sang <wsa+renesas@sang-engineering.com>
Subject: Re: [PATCH 5/6] drm/bridge: adv7511: Reuse __adv7511_power_on/off() when probing EDID
Date: Thu, 19 Jan 2017 01:05:12 +0200	[thread overview]
Message-ID: <2417180.haWkTHt9f8@avalon> (raw)
In-Reply-To: <1484614372-15342-6-git-send-email-john.stultz@linaro.org>

Hi John,

Thank you for the patch.

On Monday 16 Jan 2017 16:52:51 John Stultz wrote:
> Thus this patch changes the EDID probing logic so that we
> re-use the __adv7511_power_on/off() calls instead of duplciating
> logic.
> 
> This does change behavior slightly as it adds the HPD signal
> pulse to the EDID probe path, but Archit has had a patch to
> add HPD signal pulse to the EDID probe path before, so this
> should address the cases where that helped.

You should also mentioned that the power off path now calls 
regcache_mark_dirty(). Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> Cc: David Airlie <airlied@linux.ie>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> v4: Reworded commit message, focusing on re-use and HPD pulse
>     behavior change.
> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 12f2876..d216f61
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -573,24 +573,13 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
>  	unsigned int count;
> 
>  	/* Reading the EDID only works if the device is powered */
> -	if (!adv7511->powered) {
> -		regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
> -				   ADV7511_POWER_POWER_DOWN, 0);
> -		if (adv7511->i2c_main->irq) {
> -			regmap_write(adv7511->regmap, 
ADV7511_REG_INT_ENABLE(0),
> -				     ADV7511_INT0_EDID_READY);
> -			regmap_write(adv7511->regmap, 
ADV7511_REG_INT_ENABLE(1),
> -				     ADV7511_INT1_DDC_ERROR);
> -		}
> -		adv7511->current_edid_segment = -1;
> -	}
> +	if (!adv7511->powered)
> +		__adv7511_power_on(adv7511);
> 
>  	edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511);
> 
>  	if (!adv7511->powered)
> -		regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
> -				   ADV7511_POWER_POWER_DOWN,
> -				   ADV7511_POWER_POWER_DOWN);
> +		__adv7511_power_off(adv7511);
> 
>  	kfree(adv7511->edid);
>  	adv7511->edid = edid;

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: John Stultz <john.stultz@linaro.org>
Cc: lkml <linux-kernel@vger.kernel.org>,
	David Airlie <airlied@linux.ie>,
	Archit Taneja <architt@codeaurora.org>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>,
	Lars-Peter Clausen <lars@metafoo.de>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 5/6] drm/bridge: adv7511: Reuse __adv7511_power_on/off() when probing EDID
Date: Thu, 19 Jan 2017 01:05:12 +0200	[thread overview]
Message-ID: <2417180.haWkTHt9f8@avalon> (raw)
In-Reply-To: <1484614372-15342-6-git-send-email-john.stultz@linaro.org>

Hi John,

Thank you for the patch.

On Monday 16 Jan 2017 16:52:51 John Stultz wrote:
> Thus this patch changes the EDID probing logic so that we
> re-use the __adv7511_power_on/off() calls instead of duplciating
> logic.
> 
> This does change behavior slightly as it adds the HPD signal
> pulse to the EDID probe path, but Archit has had a patch to
> add HPD signal pulse to the EDID probe path before, so this
> should address the cases where that helped.

You should also mentioned that the power off path now calls 
regcache_mark_dirty(). Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> Cc: David Airlie <airlied@linux.ie>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> v4: Reworded commit message, focusing on re-use and HPD pulse
>     behavior change.
> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 12f2876..d216f61
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -573,24 +573,13 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
>  	unsigned int count;
> 
>  	/* Reading the EDID only works if the device is powered */
> -	if (!adv7511->powered) {
> -		regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
> -				   ADV7511_POWER_POWER_DOWN, 0);
> -		if (adv7511->i2c_main->irq) {
> -			regmap_write(adv7511->regmap, 
ADV7511_REG_INT_ENABLE(0),
> -				     ADV7511_INT0_EDID_READY);
> -			regmap_write(adv7511->regmap, 
ADV7511_REG_INT_ENABLE(1),
> -				     ADV7511_INT1_DDC_ERROR);
> -		}
> -		adv7511->current_edid_segment = -1;
> -	}
> +	if (!adv7511->powered)
> +		__adv7511_power_on(adv7511);
> 
>  	edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511);
> 
>  	if (!adv7511->powered)
> -		regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
> -				   ADV7511_POWER_POWER_DOWN,
> -				   ADV7511_POWER_POWER_DOWN);
> +		__adv7511_power_off(adv7511);
> 
>  	kfree(adv7511->edid);
>  	adv7511->edid = edid;

-- 
Regards,

Laurent Pinchart

  reply	other threads:[~2017-01-18 23:04 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-17  0:52 [PATCH 0/6 v4] adv7511 EDID probing improvements John Stultz
2017-01-17  0:52 ` John Stultz
2017-01-17  0:52 ` [PATCH 1/6] drm/bridge: adv7511: Use work_struct to defer hotplug handing to out of irq context John Stultz
2017-01-17  0:52   ` John Stultz
2017-01-17  0:52 ` [PATCH 2/6] drm/bridge: adv7511: Switch to using drm_kms_helper_hotplug_event() John Stultz
2017-01-17  0:52   ` John Stultz
2017-01-17  0:52 ` [PATCH 3/6] drm/bridge: adv7511: Enable HPD interrupts to support hotplug and improve monitor detection John Stultz
2017-01-17  0:52   ` John Stultz
2017-01-17  0:52 ` [PATCH 4/6] drm/bridge: adv7511: Rework adv7511_power_on/off() so they can be reused internally John Stultz
2017-01-17  0:52   ` John Stultz
2017-01-18 23:04   ` Laurent Pinchart
2017-01-18 23:04     ` Laurent Pinchart
2017-01-17  0:52 ` [PATCH 5/6] drm/bridge: adv7511: Reuse __adv7511_power_on/off() when probing EDID John Stultz
2017-01-17  0:52   ` John Stultz
2017-01-18 23:05   ` Laurent Pinchart [this message]
2017-01-18 23:05     ` Laurent Pinchart
2017-01-17  0:52 ` [PATCH 6/6] drm/bridge: adv7511: Re-write the i2c address before EDID probing John Stultz
2017-01-17  0:52   ` John Stultz
2017-01-18 23:06   ` Laurent Pinchart
2017-01-18 23:06     ` Laurent Pinchart
2017-01-18 23:04 ` [PATCH 0/6 v4] adv7511 EDID probing improvements Laurent Pinchart
2017-01-18 23:04   ` Laurent Pinchart
2017-01-19  5:22   ` Archit Taneja
2017-01-19  5:22     ` Archit Taneja
2017-01-19  7:37     ` John Stultz
2017-01-19  7:37       ` John Stultz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2417180.haWkTHt9f8@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=john.stultz@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=wsa+renesas@sang-engineering.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.