Re: [PATCH v7 6/9] drm/simpledrm: Add drm_panic support

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jan 04, 2024 at 05:00:50PM +0100, Jocelyn Falempe wrote:
> Add support for the drm_panic module, which displays a user-friendly
> message to the screen when a kernel panic occurs.
> 
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> ---
>  drivers/gpu/drm/tiny/simpledrm.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> index 7ce1c4617675..6dd2afee84d4 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -25,6 +25,7 @@
>  #include <drm/drm_gem_shmem_helper.h>
>  #include <drm/drm_managed.h>
>  #include <drm/drm_modeset_helper_vtables.h>
> +#include <drm/drm_panic.h>
>  #include <drm/drm_probe_helper.h>
>  
>  #define DRIVER_NAME	"simpledrm"
> @@ -985,6 +986,19 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>  	return sdev;
>  }
>  
> +static int simpledrm_get_scanout_buffer(struct drm_device *dev,
> +					struct drm_scanout_buffer *sb)
> +{
> +	struct simpledrm_device *sdev = simpledrm_device_of_dev(dev);

So I guess simpledrm is the reason why the get_scanout_buffer hook is at
the device level and not at the plane level. Even from the few drivers you
have in your series it seems very much the exception, so I'm not sure
whether that's the best design.

I guess we'll know when we see the plane iterator code with the right
locking, whether it's ok to have that in driver hooks or it's better to
pull it out into shared code.
-Sima

> +
> +	sb->width = sdev->mode.hdisplay;
> +	sb->height = sdev->mode.vdisplay;
> +	sb->pitch = sdev->pitch;
> +	sb->format = sdev->format;
> +	sb->map = sdev->screen_base;
> +	return 0;
> +}
> +
>  /*
>   * DRM driver
>   */
> @@ -1000,6 +1014,7 @@ static struct drm_driver simpledrm_driver = {
>  	.minor			= DRIVER_MINOR,
>  	.driver_features	= DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
>  	.fops			= &simpledrm_fops,
> +	.get_scanout_buffer	= simpledrm_get_scanout_buffer,
>  };
>  
>  /*
> -- 
> 2.43.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch