Re: [PATCH] drm/i915/backlight: prefer dev_priv over dev pointer

[Dave Gordon <david.s.gordon@intel.com>]

On 18/12/15 09:49, Jani Nikula wrote:
> On Fri, 18 Dec 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> On Fri, Dec 18, 2015 at 10:51:35AM +0200, Jani Nikula wrote:
>>> Use dev_priv rather than dev pointer where applicable. Remove plenty of
>>> unnecessary temp variables. No functional changes.
>>>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_panel.c | 156 ++++++++++++++-----------------------
>>>   1 file changed, 58 insertions(+), 98 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>>> index f5b4c98de608..21ee6477bf98 100644
>>> --- a/drivers/gpu/drm/i915/intel_panel.c
>>> +++ b/drivers/gpu/drm/i915/intel_panel.c
>>> @@ -461,8 +461,7 @@ static inline u32 scale_hw_to_user(struct intel_connector *connector,
>>>   static u32 intel_panel_compute_brightness(struct intel_connector *connector,
>>>   					  u32 val)
>>>   {
>>> -	struct drm_device *dev = connector->base.dev;
>>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>>> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>>   	struct intel_panel *panel = &connector->panel;
>>>
>>>   	WARN_ON(panel->backlight.max == 0);
>>> @@ -480,45 +479,40 @@ static u32 intel_panel_compute_brightness(struct intel_connector *connector,
>>>
>>>   static u32 lpt_get_backlight(struct intel_connector *connector)
>>>   {
>>> -	struct drm_device *dev = connector->base.dev;
>>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>>> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>>
>>>   	return I915_READ(BLC_PWM_PCH_CTL2) & BACKLIGHT_DUTY_CYCLE_MASK;
>>>   }
>>>
>>>   static u32 pch_get_backlight(struct intel_connector *connector)
>>>   {
>>> -	struct drm_device *dev = connector->base.dev;
>>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>>> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>>
>>>   	return I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
>>>   }
>>>
>>>   static u32 i9xx_get_backlight(struct intel_connector *connector)
>>>   {
>>> -	struct drm_device *dev = connector->base.dev;
>>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>>> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>>   	struct intel_panel *panel = &connector->panel;
>>>   	u32 val;
>>>
>>>   	val = I915_READ(BLC_PWM_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
>>> -	if (INTEL_INFO(dev)->gen < 4)
>>> +	if (INTEL_INFO(dev_priv)->gen < 4)
>>>   		val >>= 1;
>>>
>>>   	if (panel->backlight.combination_mode) {
>>>   		u8 lbpc;
>>>
>>> -		pci_read_config_byte(dev->pdev, PCI_LBPC, &lbpc);
>>> +		pci_read_config_byte(dev_priv->dev->pdev, PCI_LBPC, &lbpc);
>>
>> dev_priv->dev->pdev
>> to_pci_dev(dev_priv)
>
> Makes sense.
>
>> to_i915(connector->base.dev);
>> to_i915_from_connector(connector);
>>
>> to_i915(crtc->base.dev);
>> to_i915_from_crtc(crtc);
>
> In practise, to_i915_from_connector(&intel_connector->base) is not so
> neat. Would have to try out some patches to see what it turns out to be
> like.

Prefer crtc_to_i915() and connector_to_i915(). c.f. guc_to_i915()

Or we could eventually make a magic macro that takes any object that it 
knows about: any_to_i915(thing) :)

.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx