Hi Maxime,
On Mon, 2 May 2016 13:05:57 +0200
Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx> wrote:
> > +static void sii902x_reset(struct sii902x *sii902x)
> > +{
> > + gpiod_set_value(sii902x->reset_gpio, 1);
> > +
> > + msleep(100);
>
> Sleeping for 100ms seems like a lot. Is it required, or is it just a
> leftover from an early development?
I wish I had the answer, unfortunately I don't have the datasheet and
the implementation I based my work on [1] is sleeping 100ms.
>
> > +
> > + gpiod_set_value(sii902x->reset_gpio, 0);
> > +}
> > +
> > +static void sii902x_connector_destroy(struct drm_connector *connector)
> > +{
> > + drm_connector_unregister(connector);
> > + drm_connector_cleanup(connector);
> > +}
> > +
> > +static enum drm_connector_status
> > +sii902x_connector_detect(struct drm_connector *connector, bool force)
> > +{
> > + struct sii902x *sii902x = connector_to_sii902x(connector);
> > + unsigned int status;
> > +
> > + regmap_read(sii902x->regmap, SI902X_INT_STATUS, &status);
> > +
> > + return (status & SI902X_PLUGGED_STATUS) ?
> > + connector_status_connected : connector_status_disconnected;
> > +}
> > +
> > +static const struct drm_connector_funcs sii902x_atomic_connector_funcs = {
> > + .dpms = drm_atomic_helper_connector_dpms,
> > + .detect = sii902x_connector_detect,
> > + .fill_modes = drm_helper_probe_single_connector_modes,
> > + .destroy = sii902x_connector_destroy,
> > + .reset = drm_atomic_helper_connector_reset,
> > + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> > + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> > +};
> > +
> > +static const struct drm_connector_funcs sii902x_connector_funcs = {
> > + .dpms = drm_atomic_helper_connector_dpms,
Should be drm_atomic_helper_dpms here.
> > + .detect = sii902x_connector_detect,
> > + .fill_modes = drm_helper_probe_single_connector_modes,
> > + .destroy = sii902x_connector_destroy,
> > +};
>
> I'm guessing this is to support both atomic and !atomic drivers. I
> thought it was working automatically?
Hm, the dw-hdmi driver is providing both, but maybe it's not needed.
Daniel, any comments?
Thanks for the review.
Boris
[1]https://github.com/linux4sam/linux-at91/blob/linux-3.10-at91/drivers/hdmi/encoder-sii9022.c#L92
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
