| Message ID | 1453216500-12571-1-git-send-email-shobhit.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/19/2016 08:45 PM, Shobhit Kumar wrote: > INTEL_SOC_PMIC is loading later than I915 failing the gpiod_get and > pwm_get calls in i915. Add a retry to give time for the INTEL_SOC_PMIC > to load. This was fine till now but broke in latest kernel. Maybe load > time for the INTEL_SOC_PMIC has increased. > > Since the lookup tables for GPIO (panel enable) and PWM both are > exported by same intel_soc_pmic driver, just retrying for the driver to > load in intel_dsi_init is sufficient. By the time we come to > setup_backlight, pwm would have been exported as well. Maybe we should play with initcalls here but I was not sure how it will impact if I change it for PMIC driver. IIRC, this discussion came up from Daniel at the time of original patches also but somehow did not close decidedly and has come back as a regression. Regards Shobhit > > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> > --- > drivers/gpu/drm/i915/intel_dsi.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index 91cef35..e309ef6 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -1192,10 +1192,14 @@ void intel_dsi_init(struct drm_device *dev) > * Panel control. > */ > if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) { > + int retry = 4; > + do { > intel_dsi->gpio_panel = > gpiod_get(dev->dev, "panel", GPIOD_OUT_HIGH); > + msleep(50); > + } while (IS_ERR(intel_dsi->gpio_panel) && --retry); > > - if (IS_ERR(intel_dsi->gpio_panel)) { > + if (!retry) { > DRM_ERROR("Failed to own gpio for panel control\n"); > intel_dsi->gpio_panel = NULL; > } >
On Tue, Jan 19, 2016 at 09:00:40PM +0530, Kumar, Shobhit wrote: > On 01/19/2016 08:45 PM, Shobhit Kumar wrote: > >INTEL_SOC_PMIC is loading later than I915 failing the gpiod_get and > >pwm_get calls in i915. Add a retry to give time for the INTEL_SOC_PMIC > >to load. This was fine till now but broke in latest kernel. Maybe load > >time for the INTEL_SOC_PMIC has increased. > > > >Since the lookup tables for GPIO (panel enable) and PWM both are > >exported by same intel_soc_pmic driver, just retrying for the driver to > >load in intel_dsi_init is sufficient. By the time we come to > >setup_backlight, pwm would have been exported as well. > > Maybe we should play with initcalls here but I was not sure how it will > impact if I change it for PMIC driver. IIRC, this discussion came up from > Daniel at the time of original patches also but somehow did not close > decidedly and has come back as a regression. The correct way to handle this is to bail out with -EPROBE_DEFER. This is how this is meant to be, no amount of random retrying will fix this properly for such dynamic inter-module depencies. And yes this is going to be a world-class test for our module load unwind code. -Daniel > Regards > Shobhit > > > > >Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> > >--- > > drivers/gpu/drm/i915/intel_dsi.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > >diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > >index 91cef35..e309ef6 100644 > >--- a/drivers/gpu/drm/i915/intel_dsi.c > >+++ b/drivers/gpu/drm/i915/intel_dsi.c > >@@ -1192,10 +1192,14 @@ void intel_dsi_init(struct drm_device *dev) > > * Panel control. > > */ > > if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) { > >+ int retry = 4; > >+ do { > > intel_dsi->gpio_panel = > > gpiod_get(dev->dev, "panel", GPIOD_OUT_HIGH); > >+ msleep(50); > >+ } while (IS_ERR(intel_dsi->gpio_panel) && --retry); > > > >- if (IS_ERR(intel_dsi->gpio_panel)) { > >+ if (!retry) { > > DRM_ERROR("Failed to own gpio for panel control\n"); > > intel_dsi->gpio_panel = NULL; > > } > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 01/20/2016 01:50 AM, Daniel Vetter wrote: > On Tue, Jan 19, 2016 at 09:00:40PM +0530, Kumar, Shobhit wrote: >> On 01/19/2016 08:45 PM, Shobhit Kumar wrote: >>> INTEL_SOC_PMIC is loading later than I915 failing the gpiod_get and >>> pwm_get calls in i915. Add a retry to give time for the INTEL_SOC_PMIC >>> to load. This was fine till now but broke in latest kernel. Maybe load >>> time for the INTEL_SOC_PMIC has increased. >>> >>> Since the lookup tables for GPIO (panel enable) and PWM both are >>> exported by same intel_soc_pmic driver, just retrying for the driver to >>> load in intel_dsi_init is sufficient. By the time we come to >>> setup_backlight, pwm would have been exported as well. >> >> Maybe we should play with initcalls here but I was not sure how it will >> impact if I change it for PMIC driver. IIRC, this discussion came up from >> Daniel at the time of original patches also but somehow did not close >> decidedly and has come back as a regression. > > The correct way to handle this is to bail out with -EPROBE_DEFER. This is > how this is meant to be, no amount of random retrying will fix this > properly for such dynamic inter-module depencies. Yeah, you are right, retries are working pretty randomly. I made a quick changes all the way up to i915_driver_load returning -EPROBE_DEFER form intel_setup_outputs and it seems to work. > > And yes this is going to be a world-class test for our module load unwind > code. Well it seems to be working :) Will have a cleaned up patch for review. Also need to take care of this in the LPSS_PWM patches as well. Regards Shobhit > -Daniel > >> Regards >> Shobhit >> >>> >>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> >>> --- >>> drivers/gpu/drm/i915/intel_dsi.c | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c >>> index 91cef35..e309ef6 100644 >>> --- a/drivers/gpu/drm/i915/intel_dsi.c >>> +++ b/drivers/gpu/drm/i915/intel_dsi.c >>> @@ -1192,10 +1192,14 @@ void intel_dsi_init(struct drm_device *dev) >>> * Panel control. >>> */ >>> if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) { >>> + int retry = 4; >>> + do { >>> intel_dsi->gpio_panel = >>> gpiod_get(dev->dev, "panel", GPIOD_OUT_HIGH); >>> + msleep(50); >>> + } while (IS_ERR(intel_dsi->gpio_panel) && --retry); >>> >>> - if (IS_ERR(intel_dsi->gpio_panel)) { >>> + if (!retry) { >>> DRM_ERROR("Failed to own gpio for panel control\n"); >>> intel_dsi->gpio_panel = NULL; >>> } >>> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Wed, Jan 20, 2016 at 02:08:06PM +0530, Kumar, Shobhit wrote: > > On 01/20/2016 01:50 AM, Daniel Vetter wrote: > >On Tue, Jan 19, 2016 at 09:00:40PM +0530, Kumar, Shobhit wrote: > >>On 01/19/2016 08:45 PM, Shobhit Kumar wrote: > >>>INTEL_SOC_PMIC is loading later than I915 failing the gpiod_get and > >>>pwm_get calls in i915. Add a retry to give time for the INTEL_SOC_PMIC > >>>to load. This was fine till now but broke in latest kernel. Maybe load > >>>time for the INTEL_SOC_PMIC has increased. > >>> > >>>Since the lookup tables for GPIO (panel enable) and PWM both are > >>>exported by same intel_soc_pmic driver, just retrying for the driver to > >>>load in intel_dsi_init is sufficient. By the time we come to > >>>setup_backlight, pwm would have been exported as well. > >> > >>Maybe we should play with initcalls here but I was not sure how it will > >>impact if I change it for PMIC driver. IIRC, this discussion came up from > >>Daniel at the time of original patches also but somehow did not close > >>decidedly and has come back as a regression. > > > >The correct way to handle this is to bail out with -EPROBE_DEFER. This is > >how this is meant to be, no amount of random retrying will fix this > >properly for such dynamic inter-module depencies. > > Yeah, you are right, retries are working pretty randomly. I made a quick > changes all the way up to i915_driver_load returning -EPROBE_DEFER form > intel_setup_outputs and it seems to work. > > > > >And yes this is going to be a world-class test for our module load unwind > >code. > > Well it seems to be working :) Will have a cleaned up patch for review. Also > need to take care of this in the LPSS_PWM patches as well. You're extremely lucky then I'd say. Let's see whether CI agrees and then merge this ;-) Cheers, Daniel
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 91cef35..e309ef6 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -1192,10 +1192,14 @@ void intel_dsi_init(struct drm_device *dev) * Panel control. */ if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) { + int retry = 4; + do { intel_dsi->gpio_panel = gpiod_get(dev->dev, "panel", GPIOD_OUT_HIGH); + msleep(50); + } while (IS_ERR(intel_dsi->gpio_panel) && --retry); - if (IS_ERR(intel_dsi->gpio_panel)) { + if (!retry) { DRM_ERROR("Failed to own gpio for panel control\n"); intel_dsi->gpio_panel = NULL; }
INTEL_SOC_PMIC is loading later than I915 failing the gpiod_get and pwm_get calls in i915. Add a retry to give time for the INTEL_SOC_PMIC to load. This was fine till now but broke in latest kernel. Maybe load time for the INTEL_SOC_PMIC has increased. Since the lookup tables for GPIO (panel enable) and PWM both are exported by same intel_soc_pmic driver, just retrying for the driver to load in intel_dsi_init is sufficient. By the time we come to setup_backlight, pwm would have been exported as well. Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> --- drivers/gpu/drm/i915/intel_dsi.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)