On Fri, May 6, 2016 at 9:45 PM, Noralf Trønnes <noralf@xxxxxxxxxxx> wrote: > In the discussion following the "no more fbdev drivers" call, someone > pointed me to drm_panel. It had function hooks that I needed, so I built > tinydrm around it. If this is misusing drm_panel, it shouldn't be too > difficult to drop it. > > Actually I could just do this: > > struct tinydrm_funcs { > int (*prepare)(struct tinydrm_device *tdev); > int (*unprepare)(struct tinydrm_device *tdev); > int (*enable)(struct tinydrm_device *tdev); > int (*disable)(struct tinydrm_device *tdev); > int (*dirty)(struct drm_framebuffer *fb, void *vmem, unsigned flags, > unsigned color, struct drm_clip_rect *clips, > unsigned num_clips); > }; > > > When it comes to the simple connector, one solution could be to extend > drm_simple_display_pipe to embed a connector with (optional) modes: > > struct drm_simple_display_pipe { > - struct drm_connector *connector; > + struct drm_connector connector; > + const struct drm_display_mode *modes; > + unsigned int num_modes; > }; > > And maybe optional detect and get_modes hooks: > > struct drm_simple_display_pipe_funcs { > + enum drm_connector_status detect(struct drm_connector *connector, > + bool force); > + int (*get_modes)(struct drm_connector *connector); > }; > > int drm_simple_display_pipe_init(struct drm_device *dev, > struct drm_simple_display_pipe *pipe, > struct drm_simple_display_pipe_funcs > *funcs, > const uint32_t *formats, > unsigned int format_count, > - struct drm_connector *connector); > + int connector_type); Tbh I really like that simple_display_pipe is split after the encoder. This allows us to easily splice in a drm_bridge (which will register the drm_connector itself) with very little work. And even if there's not a full-blown bridge you might have different connectors and things. > Another solution is to create a simple connector with modes: > > struct drm_simple_connector { > struct drm_connector connector; > const struct drm_display_mode *modes; > unsigned int num_modes; > }; > > struct drm_connector *drm_simple_connector_create(struct drm_device *dev, > const struct drm_display_mode *modes, unsigned int num_modes, > int connector_type); Yeah, this makes more sense to me, but then we're kinda reinventing something like simple-panel.c with this. Otoh right now with smple_display_pipe it's not possible to wire up the panel callbacks easily, so maybe it should be a drm_bridge. Or we just leave this code in tinydrm and extract it when someone else has a need for it? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel