Re: [Intel-gfx] [PATCH 1/2] drm/i915/fbdev: add intel_fbdev_to_framebuffer() helper

[Jani Nikula <jani.nikula@intel.com>]

On Mon, 14 Feb 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Mon, Feb 14, 2022 at 05:02:06PM +0200, Jani Nikula wrote:
>> Wrap accessing struct intel_fbdev guts in a helper.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_display_debugfs.c | 6 +++---
>>  drivers/gpu/drm/i915/display/intel_fbdev.c           | 8 ++++++++
>>  drivers/gpu/drm/i915/display/intel_fbdev.h           | 7 +++++++
>>  3 files changed, 18 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> index f4de004d470f..b0bcf4d54a74 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> @@ -16,6 +16,7 @@
>>  #include "intel_dp_mst.h"
>>  #include "intel_drrs.h"
>>  #include "intel_fbc.h"
>> +#include "intel_fbdev.h"
>>  #include "intel_hdcp.h"
>>  #include "intel_hdmi.h"
>>  #include "intel_pm.h"
>> @@ -124,9 +125,8 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
>>  	struct drm_framebuffer *drm_fb;
>>  
>>  #ifdef CONFIG_DRM_FBDEV_EMULATION
>> -	if (dev_priv->fbdev && dev_priv->fbdev->helper.fb) {
>> -		fbdev_fb = to_intel_framebuffer(dev_priv->fbdev->helper.fb);
>> -
>> +	fbdev_fb = intel_fbdev_to_framebuffer(dev_priv->fbdev);
>
> The "_to_" implies to me that this is just some kind of cast,
> which it is not. So I would drop the "_to_".

Heh, I was going back and forth between with and without _to_, ended up
with this one for no particular reason. I'll flip once more. :)

>
> Otherwise the series seems fine
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks.

>
>> +	if (fbdev_fb) {
>>  		seq_printf(m, "fbcon size: %d x %d, depth %d, %d bpp, modifier 0x%llx, refcount %d, obj ",
>>  			   fbdev_fb->base.width,
>>  			   fbdev_fb->base.height,
>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
>> index 41d279db2be6..3ef683916ba6 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
>> @@ -680,3 +680,11 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
>>  	if (drm_fb_helper_restore_fbdev_mode_unlocked(&ifbdev->helper) == 0)
>>  		intel_fbdev_invalidate(ifbdev);
>>  }
>> +
>> +struct intel_framebuffer *intel_fbdev_to_framebuffer(struct intel_fbdev *fbdev)
>> +{
>> +	if (!fbdev || !fbdev->helper.fb)
>> +		return NULL;
>> +
>> +	return to_intel_framebuffer(fbdev->helper.fb);
>> +}
>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h b/drivers/gpu/drm/i915/display/intel_fbdev.h
>> index de7c84250eb5..8e86c08d544f 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.h
>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.h
>> @@ -10,6 +10,8 @@
>>  
>>  struct drm_device;
>>  struct drm_i915_private;
>> +struct intel_fbdev;
>> +struct intel_framebuffer;
>>  
>>  #ifdef CONFIG_DRM_FBDEV_EMULATION
>>  int intel_fbdev_init(struct drm_device *dev);
>> @@ -19,6 +21,7 @@ void intel_fbdev_fini(struct drm_i915_private *dev_priv);
>>  void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
>>  void intel_fbdev_output_poll_changed(struct drm_device *dev);
>>  void intel_fbdev_restore_mode(struct drm_device *dev);
>> +struct intel_framebuffer *intel_fbdev_to_framebuffer(struct intel_fbdev *fbdev);
>>  #else
>>  static inline int intel_fbdev_init(struct drm_device *dev)
>>  {
>> @@ -48,6 +51,10 @@ static inline void intel_fbdev_output_poll_changed(struct drm_device *dev)
>>  static inline void intel_fbdev_restore_mode(struct drm_device *dev)
>>  {
>>  }
>> +static inline struct intel_framebuffer *intel_fbdev_to_framebuffer(struct intel_fbdev *fbdev)
>> +{
>> +	return NULL;
>> +}
>>  #endif
>>  
>>  #endif /* __INTEL_FBDEV_H__ */
>> -- 
>> 2.30.2

-- 
Jani Nikula, Intel Open Source Graphics Center