Den 06.05.2016 16:43, skrev Thierry Reding:
On Fri, May 06, 2016 at 04:34:08PM +0200, Noralf Trønnes wrote:
Den 06.05.2016 16:15, skrev Thierry Reding:
On Fri, May 06, 2016 at 04:08:16PM +0200, Daniel Vetter wrote:
On Fri, May 06, 2016 at 04:03:47PM +0200, Thierry Reding wrote:
On Thu, May 05, 2016 at 03:24:34PM +0200, Noralf Trønnes wrote:
Add function to create a simple connector for a panel.
I'm not sure I see the usefulness of this. Typically you'd attach a
panel to an encoder/connector, in which case you already have the
connector.
Perhaps it would become more obvious why we need this if you posted
patches that show where this is used?
The other helpers give you a simple drm pipeline with plane, crtc &
encoder all baked into on drm_simple_pipeline structure. The only thing
variable you have to hook up to that is the drm_connector. And I think for
dead-simple panels avoiding the basic boilerplate in that does indeed make
some sense.
Avoiding boilerplate is good, but I have a difficult time envisioning
how you might want to use this. At the same time I'm asking myself how
we know that this helper is any good if we haven't seen it used anywhere
and actually see the boilerplate go away.
I pulled out the patches from the tinydrm patchset that would go
into drm core and helpers. I'm not very good at juggling many patches
around in various version and getting it right.
I'm doing development in the downstream Raspberry Pi repo
on 4.5 to get all the pi drivers, and then apply it on linux-next...
This is the tinydrm patch that will use it:
https://lists.freedesktop.org/archives/dri-devel/2016-April/104502.html
Extract:
+int tinydrm_display_pipe_init(struct tinydrm_device *tdev,
+ const uint32_t *formats, unsigned int format_count)
+{
+ struct drm_device *dev = tdev->base;
+ struct drm_connector *connector;
+ int ret;
+
+ connector = drm_simple_kms_panel_connector_create(dev, &tdev->panel,
+ DRM_MODE_CONNECTOR_VIRTUAL);
+ if (IS_ERR(connector))
+ return PTR_ERR(connector);
+
+ ret = drm_simple_display_pipe_init(dev, &tdev->pipe,
+ &tinydrm_display_pipe_funcs,
+ formats, format_count,
+ connector);
+
+ return ret;
+}
+EXPORT_SYMBOL(tinydrm_display_pipe_init);
drm_simple_kms_panel_connector_create() changed name when Daniel
suggested it be moved to drm_panel.c or drm_panel_helper.c.
Okay, that gives me a better understanding of where things are headed.
That said, why would these devices even need to deal with drm_panel in
the first place? Sounds to me like they are devices on some sort of
control bus that you talk to directly. And they will represent
essentially the panel with its built-in display controller.
drm_panel on the other hand was designed as an interface between display
controllers and panels, with the goal to let any display controller talk
to any panel.
While I'm sure you can support these types of simple panels using the
drm_panel infrastructure I'm not sure it's necessary, since your driver
will always talk to the panel directly, and hence the code to deal with
the panel specifics could be part of the display pipe functions.
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);
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);
Noralf.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
