Re: [PATCH 4/7] drm/pl111: Enable PL110 variant

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 31 August 2017 at 11:19, Eric Engestrom <eric.engestrom@xxxxxxxxxx> wrote:
> On Wednesday, 2017-08-30 20:07:08 +0200, Linus Walleij wrote:
>> We detect and enable the use of the PL110 variant, an earlier
>> incarnation of PL111. The only real difference is that the
>> control and interrupt enable registers have swapped place.
>> The Versatile AB and Versatile PB have a variant inbetween
>> PL110 and PL111, it is PL110 but they have already swapped the
>> two registers so those two need a bit of special handling.
>>
>> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
>> ---
>>  drivers/gpu/drm/pl111/pl111_display.c | 27 +++--------
>>  drivers/gpu/drm/pl111/pl111_drm.h     | 17 +++++++
>>  drivers/gpu/drm/pl111/pl111_drv.c     | 84 ++++++++++++++++++++++++++++++++++-
>>  3 files changed, 105 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
>> index ef86ef60aed1..6447f36c243a 100644
>> --- a/drivers/gpu/drm/pl111/pl111_display.c
>> +++ b/drivers/gpu/drm/pl111/pl111_display.c
>> @@ -201,7 +201,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
>>               break;
>>       }
>>
>> -     writel(cntl, priv->regs + CLCD_PL111_CNTL);
>> +     writel(cntl, priv->regs + priv->ctrl);
>>
>>       drm_panel_enable(priv->panel);
>>
>> @@ -219,7 +219,7 @@ void pl111_display_disable(struct drm_simple_display_pipe *pipe)
>>       drm_panel_disable(priv->panel);
>>
>>       /* Disable and Power Down */
>> -     writel(0, priv->regs + CLCD_PL111_CNTL);
>> +     writel(0, priv->regs + priv->ctrl);
>>
>>       drm_panel_unprepare(priv->panel);
>>
>> @@ -259,7 +259,7 @@ int pl111_enable_vblank(struct drm_device *drm, unsigned int crtc)
>>  {
>>       struct pl111_drm_dev_private *priv = drm->dev_private;
>>
>> -     writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + CLCD_PL111_IENB);
>> +     writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + priv->ienb);
>>
>>       return 0;
>>  }
>> @@ -268,7 +268,7 @@ void pl111_disable_vblank(struct drm_device *drm, unsigned int crtc)
>>  {
>>       struct pl111_drm_dev_private *priv = drm->dev_private;
>>
>> -     writel(0, priv->regs + CLCD_PL111_IENB);
>> +     writel(0, priv->regs + priv->ienb);
>>  }
>>
>>  static int pl111_display_prepare_fb(struct drm_simple_display_pipe *pipe,
>> @@ -412,22 +412,6 @@ int pl111_display_init(struct drm_device *drm)
>>       struct device_node *endpoint;
>>       u32 tft_r0b0g0[3];
>>       int ret;
>> -     static const u32 formats[] = {
>> -             DRM_FORMAT_ABGR8888,
>> -             DRM_FORMAT_XBGR8888,
>> -             DRM_FORMAT_ARGB8888,
>> -             DRM_FORMAT_XRGB8888,
>> -             DRM_FORMAT_BGR565,
>> -             DRM_FORMAT_RGB565,
>> -             DRM_FORMAT_ABGR1555,
>> -             DRM_FORMAT_XBGR1555,
>> -             DRM_FORMAT_ARGB1555,
>> -             DRM_FORMAT_XRGB1555,
>> -             DRM_FORMAT_ABGR4444,
>> -             DRM_FORMAT_XBGR4444,
>> -             DRM_FORMAT_ARGB4444,
>> -             DRM_FORMAT_XRGB4444,
>> -     };
>>
>>       endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
>>       if (!endpoint)
>> @@ -456,7 +440,8 @@ int pl111_display_init(struct drm_device *drm)
>>
>>       ret = drm_simple_display_pipe_init(drm, &priv->pipe,
>>                                          &pl111_display_funcs,
>> -                                        formats, ARRAY_SIZE(formats),
>> +                                        priv->variant->formats,
>> +                                        priv->variant->nformats,
>>                                          priv->connector);
>>       if (ret)
>>               return ret;
>> diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
>> index 8804af0f8997..b316a8a0fbc0 100644
>> --- a/drivers/gpu/drm/pl111/pl111_drm.h
>> +++ b/drivers/gpu/drm/pl111/pl111_drm.h
>> @@ -31,6 +31,20 @@
>>
>>  struct drm_minor;
>>
>> +/**
>> + * struct pl111_variant_data - encodes IP differences
>> + * @name: the name of this variant
>> + * @is_pl110: this is the early PL110 variant
>> + * @formats: array of supported pixel formats on this variant
>> + * @nformats: the length of the array of supported pixel formats
>> + */
>> +struct pl111_variant_data {
>> +     const char *name;
>> +     bool is_pl110;
>> +     const u32 *formats;
>> +     unsigned int nformats;
>> +};
>> +
>>  struct pl111_drm_dev_private {
>>       struct drm_device *drm;
>>
>> @@ -42,6 +56,8 @@ struct pl111_drm_dev_private {
>>       struct drm_fbdev_cma *fbdev;
>>
>>       void *regs;
>> +     u32 ienb;
>> +     u32 ctrl;
>>       /* The pixel clock (a reference to our clock divider off of CLCDCLK). */
>>       struct clk *clk;
>>       /* pl111's internal clock divider. */
>> @@ -50,6 +66,7 @@ struct pl111_drm_dev_private {
>>        * subsystem and pl111_display_enable().
>>        */
>>       spinlock_t tim2_lock;
>> +     struct pl111_variant_data *variant;
>>  };
>>
>>  int pl111_display_init(struct drm_device *dev);
>> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
>> index e66cbf202e17..f6863c0fb809 100644
>> --- a/drivers/gpu/drm/pl111/pl111_drv.c
>> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
>> @@ -206,6 +206,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>>  {
>>       struct device *dev = &amba_dev->dev;
>>       struct pl111_drm_dev_private *priv;
>> +     struct pl111_variant_data *variant = id->data;
>>       struct drm_device *drm;
>>       int ret;
>>
>> @@ -219,6 +220,33 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>>       amba_set_drvdata(amba_dev, drm);
>>       priv->drm = drm;
>>       drm->dev_private = priv;
>> +     priv->variant = variant;
>> +
>> +     /*
>> +      * The PL110 and PL111 variants have two registers
>> +      * swapped: interrupt enable and control. For this reason
>> +      * we use offsets that we can change per variant.
>> +      */
>> +     if (variant->is_pl110) {
>> +             /*
>> +              * The ARM Versatile boards are even more special:
>> +              * their PrimeCell ID say they are PL110 but the
>> +              * control and interrupt enable registers are anyway
>> +              * swapped to the PL111 order so they are not following
>> +              * the PL110 datasheet.
>> +              */
>> +             if (of_machine_is_compatible("arm,versatile-ab") ||
>> +                 of_machine_is_compatible("arm,versatile-pb")) {
>> +                     priv->ienb = CLCD_PL111_IENB;
>> +                     priv->ctrl = CLCD_PL111_CNTL;
>> +             } else {
>> +                     priv->ienb = CLCD_PL110_IENB;
>> +                     priv->ctrl = CLCD_PL110_CNTL;
>> +             }
>> +     } else {
>> +             priv->ienb = CLCD_PL111_IENB;
>> +             priv->ctrl = CLCD_PL111_CNTL;
>> +     }
>>
>>       priv->regs = devm_ioremap_resource(dev, &amba_dev->res);
>>       if (IS_ERR(priv->regs)) {
>> @@ -227,10 +255,10 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>>       }
>>
>>       /* turn off interrupts before requesting the irq */
>> -     writel(0, priv->regs + CLCD_PL111_IENB);
>> +     writel(0, priv->regs + priv->ienb);
>>
>>       ret = devm_request_irq(dev, amba_dev->irq[0], pl111_irq, 0,
>> -                            "pl111", priv);
>> +                            variant->name, priv);
>>       if (ret != 0) {
>>               dev_err(dev, "%s failed irq %d\n", __func__, ret);
>>               return ret;
>> @@ -269,10 +297,62 @@ static int pl111_amba_remove(struct amba_device *amba_dev)
>>       return 0;
>>  }
>>
>> +/*
>> + * This variant exist in early versions like the ARM Integrator
>> + * and this version lacks the 565 and 444 pixel formats.
>> + */
>> +static const u32 pl110_pixel_formats[] = {
>> +     DRM_FORMAT_ABGR8888,
>> +     DRM_FORMAT_XBGR8888,
>> +     DRM_FORMAT_ARGB8888,
>> +     DRM_FORMAT_XRGB8888,
>> +     DRM_FORMAT_ABGR1555,
>> +     DRM_FORMAT_XBGR1555,
>> +     DRM_FORMAT_ARGB1555,
>> +     DRM_FORMAT_XRGB1555,
>> +};
>> +
>> +struct pl111_variant_data pl110_variant = {
>
> I think this (and `pl111_variant` below) can be `static const`, right?
>
Static - yes, const - I don't think so.
Struct amba_id::data lacks the constness, so the const qualifier will
get discarded.

In practise everyone considers/uses ::data as const, so that could be
tweaked with separate patch.

-Emil
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux