Message ID | 1450154430-6960-1-git-send-email-gary.c.wang@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, Dec 15, 2015 at 12:40:30PM +0800, Gary Wang wrote: > The total delay of HDMI hotplug detecting with 30ms have already > been split into a resolution of 3 retries of 10ms each, for the worst > cases. But it still suffered from only waiting 10ms at most in > intel_hdmi_detect(). This patch corrects it by reading hotplug status > with 4 times at most for 30ms delay. > > v2: > - straight up to loop execution for more clear in code readability > - mdelay will replace with msleep by Daniel's new patch > > drm/i915: mdelay(10) considered harmful > > - suggest to re-evaluate try times for being compatible to old HDMI monitor > > Reviewed-by: Cooper Chiou <cooper.chiou@intel.com> > Tested-by: Gary Wang <gary.c.wang@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: Gavin Hindman <gavin.hindman@intel.com> > Cc: Sonika Jindal <sonika.jindal@intel.com> > Cc: Shashank Sharma <shashank.sharma@intel.com> > Signed-off-by: Gary Wang <gary.c.wang@intel.com> Queued for -next (including cc: fixes), thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/intel_hdmi.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > mode change 100644 => 100755 drivers/gpu/drm/i915/intel_hdmi.c > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > old mode 100644 > new mode 100755 > index 6825543..912d6c3 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -1387,17 +1387,18 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) > struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); > struct drm_i915_private *dev_priv = to_i915(connector->dev); > bool live_status = false; > - unsigned int retry = 3; > + unsigned int try; > > DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > connector->base.id, connector->name); > > intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); > > - while (!live_status && --retry) { > + for (try = 0; !live_status && try < 4; try++) { > + if (try) > + mdelay(10); > live_status = intel_digital_port_connected(dev_priv, > hdmi_to_dig_port(intel_hdmi)); > - mdelay(10); > } > > if (!live_status) > -- > 1.9.1 >
On Mon, Dec 21, 2015 at 11:29:16AM +0100, Daniel Vetter wrote: > On Tue, Dec 15, 2015 at 12:40:30PM +0800, Gary Wang wrote: > > The total delay of HDMI hotplug detecting with 30ms have already > > been split into a resolution of 3 retries of 10ms each, for the worst > > cases. But it still suffered from only waiting 10ms at most in > > intel_hdmi_detect(). This patch corrects it by reading hotplug status > > with 4 times at most for 30ms delay. > > > > v2: > > - straight up to loop execution for more clear in code readability > > - mdelay will replace with msleep by Daniel's new patch > > > > drm/i915: mdelay(10) considered harmful > > > > - suggest to re-evaluate try times for being compatible to old HDMI monitor > > > > Reviewed-by: Cooper Chiou <cooper.chiou@intel.com> > > Tested-by: Gary Wang <gary.c.wang@intel.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > Cc: Gavin Hindman <gavin.hindman@intel.com> > > Cc: Sonika Jindal <sonika.jindal@intel.com> > > Cc: Shashank Sharma <shashank.sharma@intel.com> > > Signed-off-by: Gary Wang <gary.c.wang@intel.com> > > Queued for -next (including cc: fixes), thanks for the patch. Using mdelay() or fixed to use msleep()? -Chris
On Mon, Dec 21, 2015 at 10:37:25AM +0000, Chris Wilson wrote: > On Mon, Dec 21, 2015 at 11:29:16AM +0100, Daniel Vetter wrote: > > On Tue, Dec 15, 2015 at 12:40:30PM +0800, Gary Wang wrote: > > > The total delay of HDMI hotplug detecting with 30ms have already > > > been split into a resolution of 3 retries of 10ms each, for the worst > > > cases. But it still suffered from only waiting 10ms at most in > > > intel_hdmi_detect(). This patch corrects it by reading hotplug status > > > with 4 times at most for 30ms delay. > > > > > > v2: > > > - straight up to loop execution for more clear in code readability > > > - mdelay will replace with msleep by Daniel's new patch > > > > > > drm/i915: mdelay(10) considered harmful > > > > > > - suggest to re-evaluate try times for being compatible to old HDMI monitor > > > > > > Reviewed-by: Cooper Chiou <cooper.chiou@intel.com> > > > Tested-by: Gary Wang <gary.c.wang@intel.com> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > Cc: Gavin Hindman <gavin.hindman@intel.com> > > > Cc: Sonika Jindal <sonika.jindal@intel.com> > > > Cc: Shashank Sharma <shashank.sharma@intel.com> > > > Signed-off-by: Gary Wang <gary.c.wang@intel.com> > > > > Queued for -next (including cc: fixes), thanks for the patch. > > Using mdelay() or fixed to use msleep()? msleep patch was merged already, I just had to resolve the conflict. -Daniel
Sorry for my incorrect grammar on my patch. It should be following description (mdelay-->msleep), ... v2: - straight up to loop execution for more clear in code readability - mdelay will be replaced with msleep by Daniel's new patch drm/i915: mdelay(10) considered harmful ... Gary -----Original Message----- From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter Sent: Monday, December 21, 2015 7:52 PM To: Chris Wilson <chris@chris-wilson.co.uk>; Daniel Vetter <daniel@ffwll.ch>; Wang, Gary C <gary.c.wang@intel.com>; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Correct max delay for HDMI hotplug live status checking On Mon, Dec 21, 2015 at 10:37:25AM +0000, Chris Wilson wrote: > On Mon, Dec 21, 2015 at 11:29:16AM +0100, Daniel Vetter wrote: > > On Tue, Dec 15, 2015 at 12:40:30PM +0800, Gary Wang wrote: > > > The total delay of HDMI hotplug detecting with 30ms have already > > > been split into a resolution of 3 retries of 10ms each, for the > > > worst cases. But it still suffered from only waiting 10ms at most > > > in intel_hdmi_detect(). This patch corrects it by reading hotplug > > > status with 4 times at most for 30ms delay. > > > > > > v2: > > > - straight up to loop execution for more clear in code readability > > > - mdelay will replace with msleep by Daniel's new patch > > > > > > drm/i915: mdelay(10) considered harmful > > > > > > - suggest to re-evaluate try times for being compatible to old > > > HDMI monitor > > > > > > Reviewed-by: Cooper Chiou <cooper.chiou@intel.com> > > > Tested-by: Gary Wang <gary.c.wang@intel.com> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > Cc: Gavin Hindman <gavin.hindman@intel.com> > > > Cc: Sonika Jindal <sonika.jindal@intel.com> > > > Cc: Shashank Sharma <shashank.sharma@intel.com> > > > Signed-off-by: Gary Wang <gary.c.wang@intel.com> > > > > Queued for -next (including cc: fixes), thanks for the patch. > > Using mdelay() or fixed to use msleep()? msleep patch was merged already, I just had to resolve the conflict. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c old mode 100644 new mode 100755 index 6825543..912d6c3 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1387,17 +1387,18 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); struct drm_i915_private *dev_priv = to_i915(connector->dev); bool live_status = false; - unsigned int retry = 3; + unsigned int try; DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, connector->name); intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); - while (!live_status && --retry) { + for (try = 0; !live_status && try < 4; try++) { + if (try) + mdelay(10); live_status = intel_digital_port_connected(dev_priv, hdmi_to_dig_port(intel_hdmi)); - mdelay(10); } if (!live_status)