On 01/30/2013 11:02 AM, Alexandre Courbot wrote:
> Add support for the Chunghwa CLAA101WA01A display panel.
>
> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
> ---
[...]
> +
> +#include <video/display.h>
> +
> +#define CLAA101WA01A_WIDTH 223
> +#define CLAA101WA01A_HEIGHT 125
If I remember correct, the physical size of the panel can be fetched in
EDID. If this is correct, we don't have to hard-code here.
> +
> +struct panel_claa101 {
> + struct display_entity entity;
> + struct regulator *vdd_pnl;
> + struct regulator *vdd_bl;
> + /* Enable GPIOs */
> + int pnl_enable;
> + int bl_enable;
> +};
> +
> +#define to_panel_claa101(p) container_of(p, struct panel_claa101, entity)
> +
> +static int panel_claa101_set_state(struct display_entity *entity,
> + enum display_entity_state state)
> +{
> + struct panel_claa101 *panel = to_panel_claa101(entity);
> +
> + /* OFF and STANDBY are equivalent to us */
> + if (state == DISPLAY_ENTITY_STATE_STANDBY)
> + state = DISPLAY_ENTITY_STATE_OFF;
Do we need this? The "switch" below handles the same thing already.
> +
> + switch (state) {
> + case DISPLAY_ENTITY_STATE_OFF:
> + case DISPLAY_ENTITY_STATE_STANDBY:
> + if (entity->source)
> + display_entity_set_stream(entity->source,
> + DISPLAY_ENTITY_STREAM_STOPPED);
> +
> + /* TODO error checking? */
> + gpio_set_value_cansleep(panel->bl_enable, 0);
> + usleep_range(10000, 10000);
> + regulator_disable(panel->vdd_bl);
> + usleep_range(200000, 200000);
> + gpio_set_value_cansleep(panel->pnl_enable, 0);
> + regulator_disable(panel->vdd_pnl);
> + break;
> +
> + case DISPLAY_ENTITY_STATE_ON:
> + regulator_enable(panel->vdd_pnl);
> + gpio_set_value_cansleep(panel->pnl_enable, 1);
> + usleep_range(200000, 200000);
> + regulator_enable(panel->vdd_bl);
> + usleep_range(10000, 10000);
> + gpio_set_value_cansleep(panel->bl_enable, 1);
> +
> + if (entity->source)
> + display_entity_set_stream(entity->source,
> + DISPLAY_ENTITY_STREAM_CONTINUOUS);
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int panel_claa101_get_modes(struct display_entity *entity,
> + const struct videomode **modes)
> +{
> + /* TODO get modes from EDID? */
Why not move the "nvidia,ddc" from encoder's DT to panel's DT? In that
case, you can get EDID here. I know drm has some helpers to fetch EDID
but I recall there are some other functions which has no drm
dependencies which may be suitable for you.
> + return 0;
> +}
[...]
> +
> +static int __exit panel_claa101_remove(struct platform_device *pdev)
> +{
> + struct panel_claa101 *panel = platform_get_drvdata(pdev);
> +
> + display_entity_unregister(&panel->entity);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_OF
We don't need this kind of "ifdef" in upstream kernel.
> +static struct of_device_id panel_claa101_of_match[] = {
> + { .compatible = "chunghwa,claa101wa01a", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, pwm_backlight_of_match);
What does this mean? Why we need this?
> +#else
> +#endif
> +
> +static const struct dev_pm_ops panel_claa101_dev_pm_ops = {
> +};
> +
> +static struct platform_driver panel_claa101_driver = {
> + .probe = panel_claa101_probe,
> + .remove = panel_claa101_remove,
> + .driver = {
> + .name = "panel_claa101wa01a",
> + .owner = THIS_MODULE,
> +#ifdef CONFIG_PM
> + .pm = &panel_claa101_dev_pm_ops,
If you haven't implemented this in this patch set, I suggest removing this.
> +#endif
> +#ifdef CONFIG_OF
> + .of_match_table = of_match_ptr(panel_claa101_of_match),
> +#endif
> + },
> +};
> +
> +module_platform_driver(panel_claa101_driver);
> +
> +MODULE_AUTHOR("Alexandre Courbot <acourbot@xxxxxxxxxx>");
> +MODULE_DESCRIPTION("Chunghwa CLAA101WA01A Display Panel");
> +MODULE_LICENSE("GPL");
>
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
