Hi Daniel, On Wednesday 05 June 2013 10:55:05 Daniel Vetter wrote: > On Wed, Jun 05, 2013 at 03:51:53AM +0200, Laurent Pinchart wrote: > > On Tuesday 04 June 2013 20:36:20 Daniel Vetter wrote: > > > On Tue, Jun 4, 2013 at 8:03 PM, Laurent Pinchart wrote: > > > > On Tuesday 04 June 2013 16:12:36 Daniel Vetter wrote: > > > >> On Tue, Jun 04, 2013 at 04:53:40AM +0200, Laurent Pinchart wrote: > [snip] > > > > >> > +static int rcar_du_vga_connector_get_modes(struct drm_connector > > > >> > *connector) > > > >> > +{ > > > >> > + return drm_add_modes_noedid(connector, 1280, 768); > > > >> > +} > > > >> > > > >> This (and the dummy detect function below) looks a bit funny, since > > > >> it essentially overrides the default behaviour already provided by > > > >> the crtc helpers. Until rcar has at least proper detect support for > > > >> VGA > > > > > > > > I would add that but the DDC signals are not connected on the boards I > > > > have access to :-/ > > > > > > > >> I'd just kill this and use the connector force support (and manually > > > >> adding the right resolutions). > > > > > > > > Looks like that's a candidate for better documentation... How does > > > > force support work ? > > > > > > Grep for DRM_FORCE_ON, iirc it can be set on the commandline, where you > > > can also force a specific mode. The best I could find wrt docs is the > > > kerneldoc for drm_mode_parse_command_line_for_connector. With a bit more > > > reading it looks like it's intermingled with the fbdev helper code, but > > > should be fairly easy to extract and used by your driver. > > > > It makes sense to force the connector state from command line, but I'm not > > sure if the same mechanism is the best solution here. As the driver has no > > way to know the connector state, the best we can do is guess what modes > > are supported. I can just return 0 in the get_modes handler, but then the > > core will not call drm_add_modes_noedid(), and modes will need to be > > added manually. > > > > Is your point that for a board on which the VGA connector state can't be > > detected, the user should always be responsible for adding all the modes > > supported by the VGA monitor on the command line ? > > My point is that we already have both an established code for connected > outputs without EDID to add fallback modes and means to force connectors to > certain states. Your code here seems to reinvent that wheel, so I wonder > what we should/need to improve in the common code to suit your needs. The currently available code might suit my needs, it might just be that I fail to see how to use it properly. Regarding the "code for connected outputs without EDID to add fallback modes" you're referring to, is that if (count == 0 && connector->status == connector_status_connected) count = drm_add_modes_noedid(connector, 1024, 768); in drm_helper_probe_single_connector_modes() ? That function will only be called if the connector status is connector_status_connected. There are two ways to enforce that: - returning connector_status_connected from the connector detect() operation, which seems to defeat the purpose of having connector_status_unknown completely. - setting connector->force to DRM_FORCE_ON. Are drivers allowed to do so themselves at initialization time ? Once again that seems to defeat the purpose of connector_status_unknown. > A few ideas: > - Untangling the connector forcing code from the fbdev helper so that you > can use it. > - Exposing the connector state forcing through sysfs so that it's > runtime-adjustable. My main concern here is that fbcon won't be available if we delay setting force mode until userspace is ready.. > - Adding fallback modes for connectors in the unknonw state (imo too much > risk in breaking something else). Could you please elaborate on what you thing it could break ? > Thinking about this some more I'd vote for the new sysfs file to expose > connector forcing at runtime. With that it'd boil down to 1024x756 vs. > 1280x768 for the default fallback mode. And that could be fixed with the > EDID quirk support. Although that looks like it would benefit from a > per-connector sysfs file, too ;-) -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel