Den 07.05.2016 11:59, skrev Daniel Vetter: > On Fri, May 6, 2016 at 9:45 PM, Noralf Trønnes <noralf at tronnes.org> 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?
Yes, since there's no obvious use for such a simple controller I'll just put it in tinydrm. Noralf.