From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joe Konno Subject: Re: [PATCH 4/7] drm/i915: clean up backlight conditional build Date: Wed, 06 Nov 2013 09:11:54 -0800 Message-ID: <527A785A.5000905@linux.intel.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 5F4F5F9ECE for ; Wed, 6 Nov 2013 09:12:33 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On 10/31/2013 09:55 AM, Jani Nikula wrote: > I've always felt the backlight device conditional build has been all > backwards. Make it feel right. > > Gently move things towards connector based stuff while at it. > > There should be no functional changes, except for a slight > reordering/interleaving of connector backlight and sysfs destroy calls. > > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/i915/intel_display.c | 9 +++-- > drivers/gpu/drm/i915/intel_drv.h | 2 +- > drivers/gpu/drm/i915/intel_panel.c | 65 +++++++++++++++++++--------------- > 3 files changed, 42 insertions(+), 34 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 606a594..774407d 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11007,12 +11007,11 @@ void intel_modeset_cleanup(struct drm_device *dev) > /* flush any delayed tasks or pending work */ > flush_scheduled_work(); > > - /* destroy backlight, if any, before the connectors */ > - intel_panel_destroy_backlight(dev); > - > - /* destroy the sysfs files before encoders/connectors */ > - list_for_each_entry(connector, &dev->mode_config.connector_list, head) > + /* destroy the backlight and sysfs files before encoders/connectors */ > + list_for_each_entry(connector, &dev->mode_config.connector_list, head) { > + intel_panel_destroy_backlight(connector); > drm_sysfs_connector_remove(connector); > + } > > drm_mode_config_cleanup(dev); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 1e49aa8..5548180 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -808,7 +808,7 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level, > int intel_panel_setup_backlight(struct drm_connector *connector); > void intel_panel_enable_backlight(struct intel_connector *connector); > void intel_panel_disable_backlight(struct intel_connector *connector); > -void intel_panel_destroy_backlight(struct drm_device *dev); > +void intel_panel_destroy_backlight(struct drm_connector *connector); > enum drm_connector_status intel_panel_detect(struct drm_device *dev); > > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index f161ac0..a0d13d3 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -709,16 +709,6 @@ static void intel_panel_init_backlight_regs(struct drm_device *dev) > } > } > > -static void intel_panel_init_backlight(struct drm_device *dev) > -{ > - struct drm_i915_private *dev_priv = dev->dev_private; > - > - intel_panel_init_backlight_regs(dev); > - > - dev_priv->backlight.level = intel_panel_get_backlight(dev, 0); > - dev_priv->backlight.enabled = dev_priv->backlight.level != 0; > -} > - > enum drm_connector_status > intel_panel_detect(struct drm_device *dev) > { > @@ -742,7 +732,7 @@ intel_panel_detect(struct drm_device *dev) > } > > #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE) > -static int intel_panel_update_status(struct backlight_device *bd) > +static int intel_backlight_device_update_status(struct backlight_device *bd) > { > struct intel_connector *connector = bl_get_data(bd); > struct drm_device *dev = connector->base.dev; > @@ -756,7 +746,7 @@ static int intel_panel_update_status(struct backlight_device *bd) > return 0; > } > > -static int intel_panel_get_brightness(struct backlight_device *bd) > +static int intel_backlight_device_get_brightness(struct backlight_device *bd) > { > struct intel_connector *connector = bl_get_data(bd); > struct drm_device *dev = connector->base.dev; > @@ -771,20 +761,18 @@ static int intel_panel_get_brightness(struct backlight_device *bd) > return intel_panel_get_backlight(connector->base.dev, pipe); > } > > -static const struct backlight_ops intel_panel_bl_ops = { > - .update_status = intel_panel_update_status, > - .get_brightness = intel_panel_get_brightness, > +static const struct backlight_ops intel_backlight_device_ops = { > + .update_status = intel_backlight_device_update_status, > + .get_brightness = intel_backlight_device_get_brightness, > }; > > -int intel_panel_setup_backlight(struct drm_connector *connector) > +static int intel_backlight_device_register(struct intel_connector *connector) > { > - struct drm_device *dev = connector->dev; > + struct drm_device *dev = connector->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct backlight_properties props; > unsigned long flags; > > - intel_panel_init_backlight(dev); > - > if (WARN_ON(dev_priv->backlight.device)) > return -ENODEV; > > @@ -802,9 +790,9 @@ int intel_panel_setup_backlight(struct drm_connector *connector) > } > dev_priv->backlight.device = > backlight_device_register("intel_backlight", > - connector->kdev, > - to_intel_connector(connector), > - &intel_panel_bl_ops, &props); > + connector->base.kdev, > + connector, > + &intel_backlight_device_ops, &props); > > if (IS_ERR(dev_priv->backlight.device)) { > DRM_ERROR("Failed to register backlight: %ld\n", > @@ -815,26 +803,47 @@ int intel_panel_setup_backlight(struct drm_connector *connector) > return 0; > } > > -void intel_panel_destroy_backlight(struct drm_device *dev) > +static void intel_backlight_device_unregister(struct intel_connector *connector) > { > + struct drm_device *dev = connector->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > if (dev_priv->backlight.device) { > backlight_device_unregister(dev_priv->backlight.device); > dev_priv->backlight.device = NULL; > } > } > -#else > +#else /* CONFIG_BACKLIGHT_CLASS_DEVICE */ > +static int intel_backlight_device_register(struct intel_connector *connector) > +{ > + return 0; > +} > +static void intel_backlight_device_unregister(struct intel_connector *connector) > +{ > +} > +#endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */ > + > int intel_panel_setup_backlight(struct drm_connector *connector) > { > - intel_panel_init_backlight(connector->dev); > + struct drm_device *dev = connector->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_connector *intel_connector = to_intel_connector(connector); > + > + intel_panel_init_backlight_regs(dev); > + > + dev_priv->backlight.level = intel_panel_get_backlight(dev, 0); > + dev_priv->backlight.enabled = dev_priv->backlight.level != 0; > + > + intel_backlight_device_register(intel_connector); > + > return 0; > } > > -void intel_panel_destroy_backlight(struct drm_device *dev) > +void intel_panel_destroy_backlight(struct drm_connector *connector) > { > - return; > + struct intel_connector *intel_connector = to_intel_connector(connector); > + > + intel_backlight_device_unregister(intel_connector); > } > -#endif > > int intel_panel_init(struct intel_panel *panel, > struct drm_display_mode *fixed_mode) > Tested-by: Joe Konno