Hi, On Fri, Oct 13, 2017 at 05:59:30PM +0300, Laurent Pinchart wrote: > The sdi private data structure is currently stored as a global > variable. While no platform with multiple SDI encoders currently exists > nor is planned, this doesn't comply with the kernel device model and > should thus be fixed. > > Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > ---
Reviewed-by: Sebastian Reichel <sebastian.reic...@collabora.co.uk>
-- Sebastian
> drivers/gpu/drm/omapdrm/dss/sdi.c | 122
> +++++++++++++++++++++-----------------
> 1 file changed, 69 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/sdi.c
> b/drivers/gpu/drm/omapdrm/dss/sdi.c
> index ac436826914a..a35dc51c5a6a 100644
> --- a/drivers/gpu/drm/omapdrm/dss/sdi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/sdi.c
> @@ -31,7 +31,7 @@
> #include "omapdss.h"
> #include "dss.h"
>
> -static struct {
> +struct sdi_device {
> struct platform_device *pdev;
> struct dss_device *dss;
>
> @@ -43,9 +43,9 @@ static struct {
> int datapairs;
>
> struct omap_dss_device output;
> +};
>
> - bool port_initialized;
> -} sdi;
> +#define dssdev_to_sdi(dssdev) container_of(dssdev, struct sdi_device, output)
>
> struct sdi_clk_calc_ctx {
> unsigned long pck_min, pck_max;
> @@ -77,9 +77,9 @@ static bool dpi_calc_dss_cb(unsigned long fck, void *data)
> dpi_calc_dispc_cb, ctx);
> }
>
> -static int sdi_calc_clock_div(unsigned long pclk,
> - unsigned long *fck,
> - struct dispc_clock_info *dispc_cinfo)
> +static int sdi_calc_clock_div(struct sdi_device *sdi, unsigned long pclk,
> + unsigned long *fck,
> + struct dispc_clock_info *dispc_cinfo)
> {
> int i;
> struct sdi_clk_calc_ctx ctx;
> @@ -101,7 +101,7 @@ static int sdi_calc_clock_div(unsigned long pclk,
> ctx.pck_min = 0;
> ctx.pck_max = pclk + 1000 * i * i * i;
>
> - ok = dss_div_calc(sdi.dss, pclk, ctx.pck_min,
> + ok = dss_div_calc(sdi->dss, pclk, ctx.pck_min,
> dpi_calc_dss_cb, &ctx);
> if (ok) {
> *fck = ctx.fck;
> @@ -113,26 +113,27 @@ static int sdi_calc_clock_div(unsigned long pclk,
> return -EINVAL;
> }
>
> -static void sdi_config_lcd_manager(struct omap_dss_device *dssdev)
> +static void sdi_config_lcd_manager(struct sdi_device *sdi)
> {
> - enum omap_channel channel = dssdev->dispc_channel;
> + enum omap_channel channel = sdi->output.dispc_channel;
>
> - sdi.mgr_config.io_pad_mode = DSS_IO_PAD_MODE_BYPASS;
> + sdi->mgr_config.io_pad_mode = DSS_IO_PAD_MODE_BYPASS;
>
> - sdi.mgr_config.stallmode = false;
> - sdi.mgr_config.fifohandcheck = false;
> + sdi->mgr_config.stallmode = false;
> + sdi->mgr_config.fifohandcheck = false;
>
> - sdi.mgr_config.video_port_width = 24;
> - sdi.mgr_config.lcden_sig_polarity = 1;
> + sdi->mgr_config.video_port_width = 24;
> + sdi->mgr_config.lcden_sig_polarity = 1;
>
> - dss_mgr_set_lcd_config(channel, &sdi.mgr_config);
> + dss_mgr_set_lcd_config(channel, &sdi->mgr_config);
> }
>
> static int sdi_display_enable(struct omap_dss_device *dssdev)
> {
> - struct omap_dss_device *out = &sdi.output;
> + struct sdi_device *sdi = dssdev_to_sdi(dssdev);
> + struct omap_dss_device *out = &sdi->output;
> enum omap_channel channel = dssdev->dispc_channel;
> - struct videomode *vm = &sdi.vm;
> + struct videomode *vm = &sdi->vm;
> unsigned long fck;
> struct dispc_clock_info dispc_cinfo;
> unsigned long pck;
> @@ -143,7 +144,7 @@ static int sdi_display_enable(struct omap_dss_device
> *dssdev)
> return -ENODEV;
> }
>
> - r = regulator_enable(sdi.vdds_sdi_reg);
> + r = regulator_enable(sdi->vdds_sdi_reg);
> if (r)
> goto err_reg_enable;
>
> @@ -154,11 +155,11 @@ static int sdi_display_enable(struct omap_dss_device
> *dssdev)
> /* 15.5.9.1.2 */
> vm->flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE | DISPLAY_FLAGS_SYNC_POSEDGE;
>
> - r = sdi_calc_clock_div(vm->pixelclock, &fck, &dispc_cinfo);
> + r = sdi_calc_clock_div(sdi, vm->pixelclock, &fck, &dispc_cinfo);
> if (r)
> goto err_calc_clock_div;
>
> - sdi.mgr_config.clock_info = dispc_cinfo;
> + sdi->mgr_config.clock_info = dispc_cinfo;
>
> pck = fck / dispc_cinfo.lck_div / dispc_cinfo.pck_div;
>
> @@ -172,11 +173,11 @@ static int sdi_display_enable(struct omap_dss_device
> *dssdev)
>
> dss_mgr_set_timings(channel, vm);
>
> - r = dss_set_fck_rate(sdi.dss, fck);
> + r = dss_set_fck_rate(sdi->dss, fck);
> if (r)
> goto err_set_dss_clock_div;
>
> - sdi_config_lcd_manager(dssdev);
> + sdi_config_lcd_manager(sdi);
>
> /*
> * LCLK and PCLK divisors are located in shadow registers, and we
> @@ -189,10 +190,10 @@ static int sdi_display_enable(struct omap_dss_device
> *dssdev)
> * need to care about the shadow register mechanism for pck-free. The
> * exact reason for this is unknown.
> */
> - dispc_mgr_set_clock_div(channel, &sdi.mgr_config.clock_info);
> + dispc_mgr_set_clock_div(channel, &sdi->mgr_config.clock_info);
>
> - dss_sdi_init(sdi.dss, sdi.datapairs);
> - r = dss_sdi_enable(sdi.dss);
> + dss_sdi_init(sdi->dss, sdi->datapairs);
> + r = dss_sdi_enable(sdi->dss);
> if (r)
> goto err_sdi_enable;
> mdelay(2);
> @@ -204,40 +205,45 @@ static int sdi_display_enable(struct omap_dss_device
> *dssdev)
> return 0;
>
> err_mgr_enable:
> - dss_sdi_disable(sdi.dss);
> + dss_sdi_disable(sdi->dss);
> err_sdi_enable:
> err_set_dss_clock_div:
> err_calc_clock_div:
> dispc_runtime_put();
> err_get_dispc:
> - regulator_disable(sdi.vdds_sdi_reg);
> + regulator_disable(sdi->vdds_sdi_reg);
> err_reg_enable:
> return r;
> }
>
> static void sdi_display_disable(struct omap_dss_device *dssdev)
> {
> + struct sdi_device *sdi = dssdev_to_sdi(dssdev);
> enum omap_channel channel = dssdev->dispc_channel;
>
> dss_mgr_disable(channel);
>
> - dss_sdi_disable(sdi.dss);
> + dss_sdi_disable(sdi->dss);
>
> dispc_runtime_put();
>
> - regulator_disable(sdi.vdds_sdi_reg);
> + regulator_disable(sdi->vdds_sdi_reg);
> }
>
> static void sdi_set_timings(struct omap_dss_device *dssdev,
> struct videomode *vm)
> {
> - sdi.vm = *vm;
> + struct sdi_device *sdi = dssdev_to_sdi(dssdev);
> +
> + sdi->vm = *vm;
> }
>
> static void sdi_get_timings(struct omap_dss_device *dssdev,
> struct videomode *vm)
> {
> - *vm = sdi.vm;
> + struct sdi_device *sdi = dssdev_to_sdi(dssdev);
> +
> + *vm = sdi->vm;
> }
>
> static int sdi_check_timings(struct omap_dss_device *dssdev,
> @@ -254,21 +260,21 @@ static int sdi_check_timings(struct omap_dss_device
> *dssdev,
> return 0;
> }
>
> -static int sdi_init_regulator(void)
> +static int sdi_init_regulator(struct sdi_device *sdi)
> {
> struct regulator *vdds_sdi;
>
> - if (sdi.vdds_sdi_reg)
> + if (sdi->vdds_sdi_reg)
> return 0;
>
> - vdds_sdi = devm_regulator_get(&sdi.pdev->dev, "vdds_sdi");
> + vdds_sdi = devm_regulator_get(&sdi->pdev->dev, "vdds_sdi");
> if (IS_ERR(vdds_sdi)) {
> if (PTR_ERR(vdds_sdi) != -EPROBE_DEFER)
> DSSERR("can't get VDDS_SDI regulator\n");
> return PTR_ERR(vdds_sdi);
> }
>
> - sdi.vdds_sdi_reg = vdds_sdi;
> + sdi->vdds_sdi_reg = vdds_sdi;
>
> return 0;
> }
> @@ -276,10 +282,11 @@ static int sdi_init_regulator(void)
> static int sdi_connect(struct omap_dss_device *dssdev,
> struct omap_dss_device *dst)
> {
> + struct sdi_device *sdi = dssdev_to_sdi(dssdev);
> enum omap_channel channel = dssdev->dispc_channel;
> int r;
>
> - r = sdi_init_regulator();
> + r = sdi_init_regulator(sdi);
> if (r)
> return r;
>
> @@ -325,11 +332,11 @@ static const struct omapdss_sdi_ops sdi_ops = {
> .get_timings = sdi_get_timings,
> };
>
> -static void sdi_init_output(struct platform_device *pdev)
> +static void sdi_init_output(struct sdi_device *sdi)
> {
> - struct omap_dss_device *out = &sdi.output;
> + struct omap_dss_device *out = &sdi->output;
>
> - out->dev = &pdev->dev;
> + out->dev = &sdi->pdev->dev;
> out->id = OMAP_DSS_OUTPUT_SDI;
> out->output_type = OMAP_DISPLAY_TYPE_SDI;
> out->name = "sdi.0";
> @@ -342,23 +349,28 @@ static void sdi_init_output(struct platform_device
> *pdev)
> omapdss_register_output(out);
> }
>
> -static void sdi_uninit_output(struct platform_device *pdev)
> +static void sdi_uninit_output(struct sdi_device *sdi)
> {
> - struct omap_dss_device *out = &sdi.output;
> -
> - omapdss_unregister_output(out);
> + omapdss_unregister_output(&sdi->output);
> }
>
> int sdi_init_port(struct dss_device *dss, struct platform_device *pdev,
> struct device_node *port)
> {
> + struct sdi_device *sdi;
> struct device_node *ep;
> u32 datapairs;
> int r;
>
> + sdi = kzalloc(sizeof(*sdi), GFP_KERNEL);
> + if (!sdi)
> + return -ENOMEM;
> +
> ep = of_get_next_child(port, NULL);
> - if (!ep)
> - return 0;
> + if (!ep) {
> + r = 0;
> + goto err_free;
> + }
>
> r = of_property_read_u32(ep, "datapairs", &datapairs);
> if (r) {
> @@ -366,29 +378,33 @@ int sdi_init_port(struct dss_device *dss, struct
> platform_device *pdev,
> goto err_datapairs;
> }
>
> - sdi.datapairs = datapairs;
> - sdi.dss = dss;
> + sdi->datapairs = datapairs;
> + sdi->dss = dss;
>
> of_node_put(ep);
>
> - sdi.pdev = pdev;
> + sdi->pdev = pdev;
> + port->data = sdi;
>
> - sdi_init_output(pdev);
> -
> - sdi.port_initialized = true;
> + sdi_init_output(sdi);
>
> return 0;
>
> err_datapairs:
> of_node_put(ep);
> +err_free:
> + kfree(sdi);
>
> return r;
> }
>
> void sdi_uninit_port(struct device_node *port)
> {
> - if (!sdi.port_initialized)
> + struct sdi_device *sdi = port->data;
> +
> + if (!sdi)
> return;
>
> - sdi_uninit_output(sdi.pdev);
> + sdi_uninit_output(sdi);
> + kfree(sdi);
> }
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
