Re: [PATCH v4 1/9] drm: bridge: dw-hdmi: Remove unused functions
Hi Laurent,
在 2017年03月02日 06:39, Laurent Pinchart 写道:
Most of the hdmi_phy_test_*() functions are unused. Remove them.
Signed-off-by: Laurent Pinchart
Tested-by: Nickey Yang
Best regards,
Nickey Yang
---
drivers/gpu/drm/bridge/dw-hdmi.c | 26 --
1 file changed, 26 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 9a9ec27d9e28..ce7496399ccf 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -837,32 +837,6 @@ static inline void hdmi_phy_test_clear(struct dw_hdmi
*hdmi,
HDMI_PHY_TST0_TSTCLR_MASK, HDMI_PHY_TST0);
}
-static inline void hdmi_phy_test_enable(struct dw_hdmi *hdmi,
- unsigned char bit)
-{
- hdmi_modb(hdmi, bit << HDMI_PHY_TST0_TSTEN_OFFSET,
- HDMI_PHY_TST0_TSTEN_MASK, HDMI_PHY_TST0);
-}
-
-static inline void hdmi_phy_test_clock(struct dw_hdmi *hdmi,
- unsigned char bit)
-{
- hdmi_modb(hdmi, bit << HDMI_PHY_TST0_TSTCLK_OFFSET,
- HDMI_PHY_TST0_TSTCLK_MASK, HDMI_PHY_TST0);
-}
-
-static inline void hdmi_phy_test_din(struct dw_hdmi *hdmi,
-unsigned char bit)
-{
- hdmi_writeb(hdmi, bit, HDMI_PHY_TST1);
-}
-
-static inline void hdmi_phy_test_dout(struct dw_hdmi *hdmi,
- unsigned char bit)
-{
- hdmi_writeb(hdmi, bit, HDMI_PHY_TST2);
-}
-
static bool hdmi_phy_wait_i2c_done(struct dw_hdmi *hdmi, int msec)
{
u32 val;
Re: [RFC PATCH 3/3] drm: rcar-du: Register a completion callback with VSP1
Hi Kieran,
Thank you for the patch.
On Wednesday 01 Mar 2017 13:12:56 Kieran Bingham wrote:
> Updating the state in a running VSP1 requires two interrupts from the
> VSP. Initially, the updated state will be committed - but only after the
> VSP1 has completed processing it's current frame will the new state be
> taken into account. As such, the committed state will only be 'completed'
> after an extra frame completion interrupt.
>
> Track this delay, by passing the frame flip event through the VSP
> module; It will be returned only when the frame has completed and can be
> returned to the caller.
I'll check the interrupt sequence logic tomorrow, it's a bit too late now.
Please see below for additional comments.
> Signed-off-by: Kieran Bingham
> ---
> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 8 +-
> drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 1 +-
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 34 ++-
> 3 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 7391dd95c733..0a824633a012
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -328,7 +328,7 @@ static bool rcar_du_crtc_page_flip_pending(struct
> rcar_du_crtc *rcrtc) bool pending;
>
> spin_lock_irqsave(&dev->event_lock, flags);
> - pending = rcrtc->event != NULL;
> + pending = (rcrtc->event != NULL) || (rcrtc->pending != NULL);
> spin_unlock_irqrestore(&dev->event_lock, flags);
>
> return pending;
> @@ -578,6 +578,12 @@ static irqreturn_t rcar_du_crtc_irq(int irq, void *arg)
> rcar_du_crtc_write(rcrtc, DSRCR, status & DSRCR_MASK);
>
> if (status & DSSR_FRM) {
> +
> + if (rcrtc->pending) {
> + trace_printk("VBlank loss due to VSP Overrun\n");
> + return IRQ_HANDLED;
> + }
> +
> drm_crtc_handle_vblank(&rcrtc->crtc);
> rcar_du_crtc_finish_page_flip(rcrtc);
> ret = IRQ_HANDLED;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h index a7194812997e..8374a858446a
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> @@ -46,6 +46,7 @@ struct rcar_du_crtc {
> bool started;
>
> struct drm_pending_vblank_event *event;
> + struct drm_pending_vblank_event *pending;
> wait_queue_head_t flip_wait;
>
> unsigned int outputs;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 71e70e1e0881..408375aff1a0
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -28,6 +28,26 @@
> #include "rcar_du_kms.h"
> #include "rcar_du_vsp.h"
>
> +static void rcar_du_vsp_complete(void *private, void *data)
> +{
> + struct rcar_du_crtc *crtc = (struct rcar_du_crtc *)private;
> + struct drm_device *dev = crtc->crtc.dev;
> + struct drm_pending_vblank_event *event;
> + bool match;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dev->event_lock, flags);
> + event = crtc->event;
> + crtc->event = data;
> + match = (crtc->event == crtc->pending);
> + crtc->pending = NULL;
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> +
> + /* Safety checks */
> + WARN(event, "Event lost by VSP completion callback\n");
> + WARN(!match, "Stored pending event, does not match completion\n");
I understand you want to be safe, and I assume these have never been triggered
in your tests. I'd rather replace them by a mechanism that doesn't require
passing the event to the VSP driver, and that wouldn't require adding a
pending field to the rcar_du_crtc structure. Wouldn't adding a WARN_ON(rcrtc-
>event) in rcar_du_crtc_atomic_begin() in the if (crtc->state->event) block do
the job ?
Would this get in the way of your trace_printk() debugging in
rcar_du_crtc_irq() ? Could you implement the debugging in a separate patch
without requiring to pass the event to the VSP driver ?
> +}
> +
> void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
> {
> const struct drm_display_mode *mode = &crtc->crtc.state-
>adjusted_mode;
> @@ -66,6 +86,8 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
>*/
> crtc->group->need_restart = true;
>
> + vsp1_du_register_callback(crtc->vsp->vsp, rcar_du_vsp_complete, crtc);
> +
> vsp1_du_setup_lif(crtc->vsp->vsp, mode->hdisplay, mode->vdisplay);
> }
>
> @@ -81,7 +103,17 @@ void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc)
>
> void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc)
> {
> - vsp1_du_atomic_flush(crtc->vsp->vsp, NULL);
> + struct drm_device *dev = crtc->crtc.dev;
> + struct drm_pending_vblank_event *event;
> + unsigned long flags;
> +
> + /* Move the event to the VSP, track it locally as 'pendin
Re: [RFC PATCH 2/3] v4l: vsp1: extend VSP1 module API to allow DRM callback registration
Hi Kieran,
Thank you for the patch.
On Wednesday 01 Mar 2017 13:12:55 Kieran Bingham wrote:
> To be able to perform page flips in DRM without flicker we need to be
> able to notify the rcar-du module when the VSP has completed its
> processing.
>
> To synchronise the page flip events for userspace, we move the required
> event through the VSP to track the data flow. When the frame is
> completed, the event can be returned back to the originator through the
> registered callback.
>
> We must not have bidirectional dependencies on the two components to
> maintain support for loadable modules, thus we extend the API to allow
> a callback to be registered within the VSP DRM interface.
>
> Signed-off-by: Kieran Bingham
> ---
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 2 +-
> drivers/media/platform/vsp1/vsp1_drm.c | 42 +--
> drivers/media/platform/vsp1/vsp1_drm.h | 12 -
> include/media/vsp1.h | 6 +++-
> 4 files changed, 58 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index b5bfbe50bd87..71e70e1e0881
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -81,7 +81,7 @@ void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc)
>
> void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc)
> {
> - vsp1_du_atomic_flush(crtc->vsp->vsp);
> + vsp1_du_atomic_flush(crtc->vsp->vsp, NULL);
> }
>
> /* Keep the two tables in sync. */
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> b/drivers/media/platform/vsp1/vsp1_drm.c index 8e2aa3f8e52f..743cbce48d0c
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -52,6 +52,40 @@ int vsp1_du_init(struct device *dev)
> EXPORT_SYMBOL_GPL(vsp1_du_init);
>
> /**
> + * vsp1_du_register_callback - Register VSP completion notifier callback
> + *
> + * Allow the DRM framework to register a callback with us to notify the end
> of + * processing each frame. This allows synchronisation for page
> flipping. + *
> + * @dev: the VSP device
> + * @callback: the callback function to notify the DU module
> + * @private: private structure data to pass with the callback
> + *
> + */
> +void vsp1_du_register_callback(struct device *dev,
> +void (*callback)(void *, void *),
> +void *private)
> +{
> + struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +
> + vsp1->drm->du_complete = callback;
> + vsp1->drm->du_private = private;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_du_register_callback);
As they're not supposed to change at runtime while the display is running, how
about passing the callback and private data pointer to the vsp1_du_setup_lif()
function ? Feel free to create a structure for all the parameters passed to
the function if you think we'll have too much (which would, as a side effect,
made updates to the API easier in the future as changes to the two subsystems
will be easier to decouple).
> +static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe)
> +{
> + struct vsp1_drm *drm = to_vsp1_drm(pipe);
> +
> + if (drm->du_complete && drm->active_data)
> + drm->du_complete(drm->du_private, drm->active_data);
> +
> + /* The pending frame is now active */
> + drm->active_data = drm->pending_data;
> + drm->pending_data = NULL;
> +}
I would move this function to the "Interrupt Handling" section.
> +/**
> * vsp1_du_setup_lif - Setup the output part of the VSP pipeline
> * @dev: the VSP device
> * @width: output frame width in pixels
> @@ -99,7 +133,8 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int
> width, }
>
> pipe->num_inputs = 0;
> -
> + pipe->frame_end = NULL;
You can drop this if ...
> + vsp1->drm->du_complete = NULL;
> vsp1_dlm_reset(pipe->output->dlm);
> vsp1_device_put(vsp1);
>
> @@ -196,6 +231,8 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int
> width, if (ret < 0)
> return ret;
>
> + pipe->frame_end = vsp1_du_pipeline_frame_end;
> +
... you move this to vsp1_drm_init().
> ret = media_entity_pipeline_start(&pipe->output->entity.subdev.entity,
> &pipe->pipe);
> if (ret < 0) {
> @@ -420,7 +457,7 @@ static unsigned int rpf_zpos(struct vsp1_device *vsp1,
> struct vsp1_rwpf *rpf) * vsp1_du_atomic_flush - Commit an atomic update
> * @dev: the VSP device
> */
> -void vsp1_du_atomic_flush(struct device *dev)
> +void vsp1_du_atomic_flush(struct device *dev, void *data)
> {
> struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> struct vsp1_pipeline *pipe = &vsp1->drm->pipe;
> @@ -504,6 +541,7 @@ void vsp1_du_atomic_flush(struct device *dev)
>
> vsp1_dl_list_commit(pipe->dl);
> pipe->dl = NULL;
> + vsp1->drm->pending_data = data;
>
Re: [RFC PATCH 1/3] v4l: vsp1: Register pipe with output WPF
Hi Kieran,
Thank you for the patch.
On Wednesday 01 Mar 2017 13:12:54 Kieran Bingham wrote:
> The DRM object does not register the pipe with the WPF object. This is
> used internally throughout the driver as a means of accessing the pipe.
> As such this breaks operations which require access to the pipe from WPF
> interrupts.
>
> Register the pipe inside the WPF object after it has been declared as
> the output.
>
> Signed-off-by: Kieran Bingham
> ---
> drivers/media/platform/vsp1/vsp1_drm.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> b/drivers/media/platform/vsp1/vsp1_drm.c index cd209dccff1b..8e2aa3f8e52f
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -596,6 +596,7 @@ int vsp1_drm_init(struct vsp1_device *vsp1)
> pipe->bru = &vsp1->bru->entity;
> pipe->lif = &vsp1->lif->entity;
> pipe->output = vsp1->wpf[0];
> + pipe->output->pipe = pipe;
The vsp1_irq_handler() function calls vsp1_pipeline_frame_end() with wpf-
>pipe, which is currently NULL. With this patch the function will get a non-
NULL pipeline and will thus proceed to calling vsp1_dlm_irq_frame_end():
void vsp1_pipeline_frame_end(struct vsp1_pipeline *pipe)
{
if (pipe == NULL)
return;
vsp1_dlm_irq_frame_end(pipe->output->dlm);
if (pipe->frame_end)
pipe->frame_end(pipe);
pipe->sequence++;
}
pipe->frame_end is NULL, pipe->sequence doesn't matter, but we now end up
calling vsp1_dlm_irq_frame_end(). This is a major change regarding display
list processing, yet it seems to have no effect at all.
The following commit is to blame for skipping the call to
vsp1_dlm_irq_frame_end().
commit ff7e97c94d9f7f370fe3ce2a72e85361ca22a605
Author: Laurent Pinchart
Date: Tue Jan 19 19:16:36 2016 -0200
[media] v4l: vsp1: Store pipeline pointer in rwpf
I've added a few debug print statements to vsp1_dlm_irq_frame_end(), and it
looks like we only hit the if (dlm->queued) test or none of them at all. It
looks like we've been lucky that nothing broke.
Restoring the previous behaviour should be safe, but it would be worth it
inspecting the code very carefully to make sure the logic is still correct.
I'll do it tomorrow if you don't beat me to it.
In any case, how about adding a
Fixes: ff7e97c94d9f ("[media] v4l: vsp1: Store pipeline pointer in rwpf")
line ?
> return 0;
> }
--
Regards,
Laurent Pinchart
RE: [PATCH v4 1/3] watchdog: add rza_wdt driver
Hello Guenter,
Thank you for your review!
On Thursday, March 02, 2017, Guenter Roeck wrote:
> > +/*
> > + * Renesas RZ/A Series WDT Driver
> > + *
> > + * Copyright (C) 2017 Renesas Electronics America, Inc.
> > + * Copyright (C) 2017 Chris Brandt
> > + *
> > + * This file is subject to the terms and conditions of the GNU
> > +General Public
> > + * License. See the file "COPYING" in the main directory of this
> > +archive
> > + * for more details.
> > + *
> > + *
>
> The above two lines are unnecessary.
OK.
#I'll assume you mean take out just the last sentence (2 lines), not both
sentences (all 3 lines).
> > +/* Watchdog Timer Registers */
> > +#define WTCSR 0
> > +#define WTCSR_MAGIC 0xA500
> > +#define WTSCR_WT (1<<6)
> > +#define WTSCR_TME (1<<5)
>
> BIT()
OK.
> > +#define WTSCR_CKS(i) i
>
> (i)
OK.
> > +#define WRCSR_CLEAR_WOVF 0xA500 /* special value */
>
> Please use
> #define SOMETHINGvalue
> throughout and make sure value is aligned.
OK.
> > + rate = clk_get_rate(priv->clk);
> > + if (!rate)
> > + return -ENOENT;
> > +
> > + /* Assume slowest clock rate possible (CKS=7) */
> > + rate /= 16384;
> > +
>
> The rate check should probably be here to avoid situations where rate <
> 16384.
Do I need that if it's technically not possible to have a 'rate' less than
25MHz?
These watchdogs HW are always feed directly from the peripheral clock and there
is no such thing as a 16kHz peripheral block an any Renesas SoC.
> > + priv->wdev.info = &rza_wdt_ident,
> > + priv->wdev.ops = &rza_wdt_ops,
> > + priv->wdev.parent = &pdev->dev;
> > +
> > + /*
> > +* Since the max possible timeout of our 8-bit count register is
> less
> > +* than a second, we must use max_hw_heartbeat_ms.
> > +*/
> > + priv->wdev.max_hw_heartbeat_ms = (1000 * U8_MAX)/rate;
>
> space before and after /
OK.
#Funny because checkpatch.pl said it didn't like a space on one side but
not the other, so I choose no spaces and it was happy. I'm way below 80
characters for that line so it doesn't matter to me.
> > + dev_info(&pdev->dev, "max hw timeout of %dms\n",
> > +priv->wdev.max_hw_heartbeat_ms);
>
> dev_dbg ?
OK.
#I guess we don't need to see that info on every boot.
> > +
> > + priv->wdev.min_timeout = 1;
> > + priv->wdev.timeout = DEFAULT_TIMEOUT;
> > +
>
> Add watchdog_init_timeout(&priv->wdev, 0, &pdev->dev); to optionally get
> the timeout from dt ?
OK. Good idea.
> > + platform_set_drvdata(pdev, priv);
> > + watchdog_set_drvdata(&priv->wdev, priv);
> > +
> > + ret = watchdog_register_device(&priv->wdev);
>
> devm_watchdog_register_device()
OK.
> > + if (ret < 0)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static int rza_wdt_remove(struct platform_device *pdev) {
> > + struct rza_wdt *priv = platform_get_drvdata(pdev);
> > +
> > + watchdog_unregister_device(&priv->wdev);
> > + iounmap(priv->base);
>
> iounmap is unnecessary (and wrong).
Anything mapped with devm_ioremap_resource() automatically gets unmapped
when the drive gets unloaded?
I didn't know that.
I'll wait till the end of the day to see if anyone finds anything else, and
then I'll send out a v5.
Chris
RE: [PATCH 4/7] arm: dts: r7s72100: Add pin controller node
Hi Jacopo,
On Tuesday, February 21, 2017, jacopo mondi wrote:
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + gpio-ranges = <&pinctrl 0 0 16>;
>
> Not all ports have 16 pins available.
> This is one of the differences between different RZ/A1 SoC versions
> (RZ/A1H, RZ/A1L etc)
>
> I'll change the number of pins to the actual available ones on each port
> for r7s72100 (RZ/A1H)
But wait, what if someone wants to use a RZ/A1L???
I don't want to make a separate dtsi file for RZ/A1L.
Is it possible to change the number of port pins in the board dts file?
For example:
RZ/A1H: P5_0 - P5_10
RZ/A1L: P5_0 - P5_16
So, in a rza1l-board.dts file I would put:
&port5 {
gpio-ranges = <&pinctrl 0 80 16>;
}
Will this work?
Chris
RE: Renesas RZ/A1 pin and gpio controller
Hi Jacopo, On Monday, February 20, 2017, Jacopo Mondi wrote: > Hello, >this is the first submission of combined GPIO and pin controller driver > for Renesas RZ/A1 SoC. > > Compared to my RFC series on the same subject, this new implementation > supports a single SoC. If more devices with a similar pin controller will > arrive later, we will consider supporting them through this driver. > > The series adds the driver itself and register the pincontroller in dtsi. > The pin controller hardware supports 12 ports, each of them is also a gpio > controller. > > The series makes use of pinctrl and pinmux generic function currently > available in Linus Walleij's linux-pinctrl.git tree. > > Testing done veryfing functionalities of hardware modules enabled in > device tree (SCIF2 for serial output, RIIC for accessing an internal > eeprom chip and user visible leds). > Gpio have been also verified using a i2c-gpio device in place of the > native RIIC one to access the same eeprom device. Functionally, I tested these patches on an RZ/A1H RSK board (after modifying the DT for the RSK vs the GENMAI board). The following worked fine: * SCIF * I2C * SDHI * Ethernet (The trick is you have to know what pins need to be set as bi-directional) I will look over the code and provide feedback separately. Cheers Chris
RE: [PATCH v4 1/3] watchdog: add rza_wdt driver
On Thursday, March 02, 2017, Guenter Roeck wrote:
> > > > > The rate check should probably be here to avoid situations where
> > > > > rate < 16384.
> > > >
> > > > Do I need that if it's technically not possible to have a 'rate'
> > > > less
> > > than 25MHz?
> > > >
> > > > These watchdogs HW are always feed directly from the peripheral
> > > > clock and there is no such thing as a 16kHz peripheral block an
> > > > any Renesas
> > > SoC.
> > > >
> > > Following that line of argument, can clk_get_rate() ever return 0 ?
> >
> > In the DT binding, it says that a clock source is required to be present.
> >
> > If the user leaves out the "clocks =", then devm_clk_get will fail.
> >
> > If the user puts in some crazy value for "clocks = ", then maybe you
> > could get
> > 0 (assuming there is a valid clock node they made by themselves
> > somewhere that runs at 0Hz).
> > But in that extreme case, I think they deserve to have it crash and
> > burn because who knows what they are doing.
> >
>
> But then there could also be a clock source with a rate of less than 16
> kHz, as wrong as it may be ?
>
> Anyway, I disagree about the crash and burn. It isn't as if this would be
> really fatal except for the watchdog driver. Bad data in devicetree should
> not result in a system crash.
OK. I will put the check in. Something like:
rate = clk_get_rate(priv->clk);
if (rate < 16384) {
dev_err(&pdev->dev, "invalid clock specified\n");
return -ENOENT;
}
> > > That is the point of devm_ functions. It also means that you won't
> > > need a remove function if you also use devm_watchdog_register_device().
> >
> > OK.
> > I see that only 1 driver is using devm_watchdog_register_device
> > (wdat_wdt.c), so maybe that is a new method.
> >
> Yes, it is quite new. Still, you are a bit behind. I count 19 users in the
> mainline kernel.
OK, I see now.
Thank you,
Chris
RE: [PATCH v4 1/3] watchdog: add rza_wdt driver
On Thursday, March 02, 2017, Guenter Roeck worte:
> > > The above two lines are unnecessary.
> >
> > OK.
> >
> > #I'll assume you mean take out just the last sentence (2 lines), not
> > both sentences (all 3 lines).
> >
> The two empty lines.
Ooops! That makes more sense.
> > > > + rate = clk_get_rate(priv->clk);
> > > > + if (!rate)
> > > > + return -ENOENT;
> > > > +
> > > > + /* Assume slowest clock rate possible (CKS=7) */
> > > > + rate /= 16384;
> > > > +
> > >
> > > The rate check should probably be here to avoid situations where
> > > rate < 16384.
> >
> > Do I need that if it's technically not possible to have a 'rate' less
> than 25MHz?
> >
> > These watchdogs HW are always feed directly from the peripheral clock
> > and there is no such thing as a 16kHz peripheral block an any Renesas
> SoC.
> >
> Following that line of argument, can clk_get_rate() ever return 0 ?
In the DT binding, it says that a clock source is required to be present.
If the user leaves out the "clocks =", then devm_clk_get will fail.
If the user puts in some crazy value for "clocks = ", then maybe you could get
0 (assuming there is a valid clock node they made by themselves somewhere that
runs at 0Hz).
But in that extreme case, I think they deserve to have it crash and burn because
who knows what they are doing.
> > > > + priv->wdev.max_hw_heartbeat_ms = (1000 * U8_MAX)/rate;
> > >
> > > space before and after /
> >
> > OK.
> > #Funny because checkpatch.pl said it didn't like a space on one side
> > but not the other, so I choose no spaces and it was happy. I'm way
> > below 80 characters for that line so it doesn't matter to me.
> >
>
> That would be a bug in checkpatch. coding style, chapter 3.1, still
> applies.
> Or at least I hope so.
OK. Thank you for the clarification.
> > > > + if (ret < 0)
> > > > + return ret;
> > > > +
> > > > + return 0;
>
> Also just
> return ret;
OK.
> > > > +static int rza_wdt_remove(struct platform_device *pdev) {
> > > > + struct rza_wdt *priv = platform_get_drvdata(pdev);
> > > > +
> > > > + watchdog_unregister_device(&priv->wdev);
> > > > + iounmap(priv->base);
> > >
> > > iounmap is unnecessary (and wrong).
> >
> > Anything mapped with devm_ioremap_resource() automatically gets
> > unmapped when the drive gets unloaded?
>
> That is the point of devm_ functions. It also means that you won't need a
> remove function if you also use devm_watchdog_register_device().
OK.
I see that only 1 driver is using devm_watchdog_register_device (wdat_wdt.c), so
maybe that is a new method.
Thank you,
Chris
Re: [PATCH v4 1/3] watchdog: add rza_wdt driver
On Thu, Mar 02, 2017 at 06:22:17PM +, Chris Brandt wrote:
> On Thursday, March 02, 2017, Guenter Roeck wrote:
> > > > > > The rate check should probably be here to avoid situations where
> > > > > > rate < 16384.
> > > > >
> > > > > Do I need that if it's technically not possible to have a 'rate'
> > > > > less
> > > > than 25MHz?
> > > > >
> > > > > These watchdogs HW are always feed directly from the peripheral
> > > > > clock and there is no such thing as a 16kHz peripheral block an
> > > > > any Renesas
> > > > SoC.
> > > > >
> > > > Following that line of argument, can clk_get_rate() ever return 0 ?
> > >
> > > In the DT binding, it says that a clock source is required to be present.
> > >
> > > If the user leaves out the "clocks =", then devm_clk_get will fail.
> > >
> > > If the user puts in some crazy value for "clocks = ", then maybe you
> > > could get
> > > 0 (assuming there is a valid clock node they made by themselves
> > > somewhere that runs at 0Hz).
> > > But in that extreme case, I think they deserve to have it crash and
> > > burn because who knows what they are doing.
> > >
> >
> > But then there could also be a clock source with a rate of less than 16
> > kHz, as wrong as it may be ?
> >
> > Anyway, I disagree about the crash and burn. It isn't as if this would be
> > really fatal except for the watchdog driver. Bad data in devicetree should
> > not result in a system crash.
>
> OK. I will put the check in. Something like:
>
> rate = clk_get_rate(priv->clk);
> if (rate < 16384) {
> dev_err(&pdev->dev, "invalid clock specified\n");
> return -ENOENT;
> }
>
>
Sounds good. Maybe display the wrong rate as well ?
Not a strong opinion though.
Thanks,
Guenter
> > > > That is the point of devm_ functions. It also means that you won't
> > > > need a remove function if you also use devm_watchdog_register_device().
> > >
> > > OK.
> > > I see that only 1 driver is using devm_watchdog_register_device
> > > (wdat_wdt.c), so maybe that is a new method.
> > >
> > Yes, it is quite new. Still, you are a bit behind. I count 19 users in the
> > mainline kernel.
>
> OK, I see now.
>
>
> Thank you,
> Chris
[PATCH v4 1/4] devicetree/bindings: display: bridge: Add LVDS encoder DT bindings
The DT bindings support parallel to LVDS encoders that don't require any
configuration, similarly to the dumb VGA DAC DT bindings.
Signed-off-by: Laurent Pinchart
Acked-by: Rob Herring
---
.../bindings/display/bridge/lvds-transmitter.txt | 64 ++
1 file changed, 64 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
diff --git
a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
new file mode 100644
index ..fd39ad34c383
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -0,0 +1,64 @@
+Parallel to LVDS Encoder
+
+
+This binding supports the parallel to LVDS encoders that don't require any
+configuration.
+
+LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. Multiple
+incompatible data link layers have been used over time to transmit image data
+to LVDS panels. This binding targets devices compatible with the following
+specifications only.
+
+[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, February
+1999 (Version 1.0), Japan Electronic Industry Development Association (JEIDA)
+[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
+Semiconductor
+[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
+Electronics Standards Association (VESA)
+
+Those devices have been marketed under the FPD-Link and FlatLink brand names
+among others.
+
+
+Required properties:
+
+- compatible: Must be "lvds-encoder"
+
+Required nodes:
+
+This device has two video ports. Their connections are modeled using the OF
+graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for parallel input
+- Video port 1 for LVDS output
+
+
+Example
+---
+
+lvds-encoder {
+ compatible = "lvds-encoder";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ lvds_enc_in: endpoint {
+ remote-endpoint = <&display_out_rgb>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ lvds_enc_out: endpoint {
+ remote-endpoint = <&lvds_panel_in>;
+ };
+ };
+ };
+};
--
Regards,
Laurent Pinchart
Re: [PATCH v4 8/9] drm: bridge: dw-hdmi: Remove device type from platform data
Hi Laurent,
On 01-03-2017 22:39, Laurent Pinchart wrote:
> From: Kieran Bingham
>
> The device type isn't used anymore now that workarounds and PHY-specific
> operations are performed based on version information read at runtime.
> Remove it.
>
> Signed-off-by: Kieran Bingham
> Signed-off-by: Laurent Pinchart
Reviewed-by: Jose Abreu
Best regards,
Jose Miguel Abreu
> ---
> drivers/gpu/drm/bridge/dw-hdmi.c| 2 --
> drivers/gpu/drm/imx/dw_hdmi-imx.c | 2 --
> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 1 -
> include/drm/bridge/dw_hdmi.h| 7 ---
> 4 files changed, 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index b835d81bb471..132c00685796 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -127,7 +127,6 @@ struct dw_hdmi {
> struct drm_connector connector;
> struct drm_bridge bridge;
>
> - enum dw_hdmi_devtype dev_type;
> unsigned int version;
>
> struct platform_device *audio;
> @@ -2014,7 +2013,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
>
> hdmi->plat_data = plat_data;
> hdmi->dev = dev;
> - hdmi->dev_type = plat_data->dev_type;
> hdmi->sample_rate = 48000;
> hdmi->disabled = true;
> hdmi->rxsense = true;
> diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c
> b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> index f645275e6e63..f039641070ac 100644
> --- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
> +++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> @@ -175,7 +175,6 @@ static struct dw_hdmi_plat_data imx6q_hdmi_drv_data = {
> .mpll_cfg = imx_mpll_cfg,
> .cur_ctr= imx_cur_ctr,
> .phy_config = imx_phy_config,
> - .dev_type = IMX6Q_HDMI,
> .mode_valid = imx6q_hdmi_mode_valid,
> };
>
> @@ -183,7 +182,6 @@ static struct dw_hdmi_plat_data imx6dl_hdmi_drv_data = {
> .mpll_cfg = imx_mpll_cfg,
> .cur_ctr = imx_cur_ctr,
> .phy_config = imx_phy_config,
> - .dev_type = IMX6DL_HDMI,
> .mode_valid = imx6dl_hdmi_mode_valid,
> };
>
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index a6d4a0236e8f..d53827413996 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -237,7 +237,6 @@ static const struct dw_hdmi_plat_data
> rockchip_hdmi_drv_data = {
> .mpll_cfg = rockchip_mpll_cfg,
> .cur_ctr= rockchip_cur_ctr,
> .phy_config = rockchip_phy_config,
> - .dev_type = RK3288_HDMI,
> };
>
> static const struct of_device_id dw_hdmi_rockchip_dt_ids[] = {
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index dd330259a3dc..545f04fae3b6 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -21,12 +21,6 @@ enum {
> DW_HDMI_RES_MAX,
> };
>
> -enum dw_hdmi_devtype {
> - IMX6Q_HDMI,
> - IMX6DL_HDMI,
> - RK3288_HDMI,
> -};
> -
> enum dw_hdmi_phy_type {
> DW_HDMI_PHY_DWC_HDMI_TX_PHY = 0x00,
> DW_HDMI_PHY_DWC_MHL_PHY_HEAC = 0xb2,
> @@ -65,7 +59,6 @@ struct dw_hdmi_phy_ops {
> };
>
> struct dw_hdmi_plat_data {
> - enum dw_hdmi_devtype dev_type;
> enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
> struct drm_display_mode *mode);
>
Re: [PATCH v4 1/3] watchdog: add rza_wdt driver
On Thu, Mar 02, 2017 at 03:38:07PM +, Chris Brandt wrote:
> Hello Guenter,
>
> Thank you for your review!
>
>
> On Thursday, March 02, 2017, Guenter Roeck wrote:
> > > +/*
> > > + * Renesas RZ/A Series WDT Driver
> > > + *
> > > + * Copyright (C) 2017 Renesas Electronics America, Inc.
> > > + * Copyright (C) 2017 Chris Brandt
> > > + *
> > > + * This file is subject to the terms and conditions of the GNU
> > > +General Public
> > > + * License. See the file "COPYING" in the main directory of this
> > > +archive
> > > + * for more details.
> > > + *
> > > + *
> >
> > The above two lines are unnecessary.
>
> OK.
>
> #I'll assume you mean take out just the last sentence (2 lines), not both
> sentences (all 3 lines).
>
The two empty lines.
>
> > > +/* Watchdog Timer Registers */
> > > +#define WTCSR 0
> > > +#define WTCSR_MAGIC 0xA500
> > > +#define WTSCR_WT (1<<6)
> > > +#define WTSCR_TME (1<<5)
> >
> > BIT()
>
> OK.
>
> > > +#define WTSCR_CKS(i) i
> >
> > (i)
>
> OK.
>
>
> > > +#define WRCSR_CLEAR_WOVF 0xA500 /* special value */
> >
> > Please use
> > #define SOMETHINGvalue
> > throughout and make sure value is aligned.
>
> OK.
>
> > > + rate = clk_get_rate(priv->clk);
> > > + if (!rate)
> > > + return -ENOENT;
> > > +
> > > + /* Assume slowest clock rate possible (CKS=7) */
> > > + rate /= 16384;
> > > +
> >
> > The rate check should probably be here to avoid situations where rate <
> > 16384.
>
> Do I need that if it's technically not possible to have a 'rate' less than
> 25MHz?
>
> These watchdogs HW are always feed directly from the peripheral clock and
> there
> is no such thing as a 16kHz peripheral block an any Renesas SoC.
>
Following that line of argument, can clk_get_rate() ever return 0 ?
>
> > > + priv->wdev.info = &rza_wdt_ident,
> > > + priv->wdev.ops = &rza_wdt_ops,
> > > + priv->wdev.parent = &pdev->dev;
> > > +
> > > + /*
> > > + * Since the max possible timeout of our 8-bit count register is
> > less
> > > + * than a second, we must use max_hw_heartbeat_ms.
> > > + */
> > > + priv->wdev.max_hw_heartbeat_ms = (1000 * U8_MAX)/rate;
> >
> > space before and after /
>
> OK.
> #Funny because checkpatch.pl said it didn't like a space on one side but
> not the other, so I choose no spaces and it was happy. I'm way below 80
> characters for that line so it doesn't matter to me.
>
That would be a bug in checkpatch. coding style, chapter 3.1, still applies.
Or at least I hope so.
>
> > > + dev_info(&pdev->dev, "max hw timeout of %dms\n",
> > > + priv->wdev.max_hw_heartbeat_ms);
> >
> > dev_dbg ?
>
> OK.
> #I guess we don't need to see that info on every boot.
>
>
> > > +
> > > + priv->wdev.min_timeout = 1;
> > > + priv->wdev.timeout = DEFAULT_TIMEOUT;
> > > +
> >
> > Add watchdog_init_timeout(&priv->wdev, 0, &pdev->dev); to optionally get
> > the timeout from dt ?
>
> OK. Good idea.
>
> > > + platform_set_drvdata(pdev, priv);
> > > + watchdog_set_drvdata(&priv->wdev, priv);
> > > +
> > > + ret = watchdog_register_device(&priv->wdev);
> >
> > devm_watchdog_register_device()
>
> OK.
>
>
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + return 0;
Also just
return ret;
> > > +}
> > > +
> > > +static int rza_wdt_remove(struct platform_device *pdev) {
> > > + struct rza_wdt *priv = platform_get_drvdata(pdev);
> > > +
> > > + watchdog_unregister_device(&priv->wdev);
> > > + iounmap(priv->base);
> >
> > iounmap is unnecessary (and wrong).
>
> Anything mapped with devm_ioremap_resource() automatically gets unmapped
> when the drive gets unloaded?
That is the point of devm_ functions. It also means that you won't need
a remove function if you also use devm_watchdog_register_device().
Guenter
Re: [PATCH v4 1/3] watchdog: add rza_wdt driver
On Thu, Mar 02, 2017 at 05:31:18PM +, Chris Brandt wrote:
> On Thursday, March 02, 2017, Guenter Roeck worte:
> > > > The above two lines are unnecessary.
> > >
> > > OK.
> > >
> > > #I'll assume you mean take out just the last sentence (2 lines), not
> > > both sentences (all 3 lines).
> > >
> > The two empty lines.
>
> Ooops! That makes more sense.
>
>
> > > > > + rate = clk_get_rate(priv->clk);
> > > > > + if (!rate)
> > > > > + return -ENOENT;
> > > > > +
> > > > > + /* Assume slowest clock rate possible (CKS=7) */
> > > > > + rate /= 16384;
> > > > > +
> > > >
> > > > The rate check should probably be here to avoid situations where
> > > > rate < 16384.
> > >
> > > Do I need that if it's technically not possible to have a 'rate' less
> > than 25MHz?
> > >
> > > These watchdogs HW are always feed directly from the peripheral clock
> > > and there is no such thing as a 16kHz peripheral block an any Renesas
> > SoC.
> > >
> > Following that line of argument, can clk_get_rate() ever return 0 ?
>
> In the DT binding, it says that a clock source is required to be present.
>
> If the user leaves out the "clocks =", then devm_clk_get will fail.
>
> If the user puts in some crazy value for "clocks = ", then maybe you could get
> 0 (assuming there is a valid clock node they made by themselves somewhere that
> runs at 0Hz).
> But in that extreme case, I think they deserve to have it crash and burn
> because
> who knows what they are doing.
>
But then there could also be a clock source with a rate of less than 16 kHz, as
wrong as it may be ?
Anyway, I disagree about the crash and burn. It isn't as if this would be really
fatal except for the watchdog driver. Bad data in devicetree should not result
in a system crash.
>
> > > > > + priv->wdev.max_hw_heartbeat_ms = (1000 * U8_MAX)/rate;
> > > >
> > > > space before and after /
> > >
> > > OK.
> > > #Funny because checkpatch.pl said it didn't like a space on one side
> > > but not the other, so I choose no spaces and it was happy. I'm way
> > > below 80 characters for that line so it doesn't matter to me.
> > >
> >
> > That would be a bug in checkpatch. coding style, chapter 3.1, still
> > applies.
> > Or at least I hope so.
>
> OK. Thank you for the clarification.
>
>
> > > > > + if (ret < 0)
> > > > > + return ret;
> > > > > +
> > > > > + return 0;
> >
> > Also just
> > return ret;
>
> OK.
>
>
> > > > > +static int rza_wdt_remove(struct platform_device *pdev) {
> > > > > + struct rza_wdt *priv = platform_get_drvdata(pdev);
> > > > > +
> > > > > + watchdog_unregister_device(&priv->wdev);
> > > > > + iounmap(priv->base);
> > > >
> > > > iounmap is unnecessary (and wrong).
> > >
> > > Anything mapped with devm_ioremap_resource() automatically gets
> > > unmapped when the drive gets unloaded?
> >
> > That is the point of devm_ functions. It also means that you won't need a
> > remove function if you also use devm_watchdog_register_device().
>
> OK.
> I see that only 1 driver is using devm_watchdog_register_device (wdat_wdt.c),
> so
> maybe that is a new method.
>
Yes, it is quite new. Still, you are a bit behind. I count 19 users
in the mainline kernel.
Guenter
Re: [PATCH v2 0/2] arm64: dts: renesas: Upgrade to PSCI v1.0 to support Suspend-to-RAM
On Fri, Feb 24, 2017 at 02:49:12PM +0100, Geert Uytterhoeven wrote: > Hi Simon, Magnus, > > This patch series upgrades the version of the PSCI firmware, as > advertised in DT on Renesas R-Car Gen3 systems, from v0.2 to v1.0. > > Note that this series does not have any runtime effects: as of PSCI > v0.2, PSCI provides a VERSION API. Hence Linux always detected the > actual version, regardless of what was stated in DT, and enabled v1.0 > features like system suspend: > > PSCIv1.0 detected in firmware. > > However, it is better to have the real hardware^H^H^H^H^H^H^H^Hfirmware > description in DT. > > Changes compared to v1: > - Keep "arm,psci-0.2". > > Tested on Salvator-X (both H3 and M3-W) with firmware v2.16.0, but I do > have old boot logs showing that PSCIv1.0 has been detected for a long > time. > > Thanks for applying! Thanks, I have queued these up for v4.12.
Re: [PATCH v4 7/9] drm: bridge: dw-hdmi: Add support for custom PHY configuration
Hi Jose, On Thursday 02 Mar 2017 14:50:02 Jose Abreu wrote: > On 02-03-2017 13:41, Laurent Pinchart wrote: > >> Hmm, this is kind of confusing. Why do you need a phy_ops and > >> then a separate configure_phy function? Can't we just use phy_ops > >> always? If its external phy then it would need to implement all > >> phy_ops functions. > > > > The phy_ops are indeed meant to support vendor PHYs. The .configure_phy() > > operation is meant to support Synopsys PHYs that don't comply with the > > HDMI TX MHL and 3D PHYs I2C register layout and thus need a different > > configure function, but can share the other operations with the HDMI TX > > MHL and 3D PHYs. Ideally I'd like to move that code to the dw-hdmi core, > > but at the moment I don't have enough information to decide whether the > > register layout corresponds to the standard Synopsys HDMI TX 2.0 PHY or > > if it has been modified by the vendor. Once we'll have a second SoC using > > the same PHY we should have more information to consolidate the code. Of > > course, if you can share the HDMI TX 2.0 PHY datasheet, I won't mind > > reworking the code now ;-) > > Well, if I want to keep my job I can't share the datasheet, and I > do want to keep my job :) That's so selfish :-D > As far as I know this shouldn't change much. I already used (it > was like a year ago) the dw-hdmi driver in a HDMI TX 2.0 PHY. I really wonder what exactly has been integrated in the Renesas R-Car Gen3 SoC. The PHY is certainly reported as an HDMI TX 2.0 PHY, but the registers seem different compared to the 2.0 PHY you've tested. > But I am not following your logic here, sorry: You are mentioning a > custom phy, right? Custom is probably a bad name. From what I've been told it's a standard Synopsys PHY, but I can't rule out vendor-specific customizations. > If its custom then it should implement its own phy_ops. And I don't think > that phy logic should go into core, this should all be extracted because I > really believe it will make the code easier to read. Imagine this > organization: > > Folders/Files: > drm/bridge/dw-hdmi/dw-hdmi-core.c > drm/bridge/dw-hdmi/dw-hdmi-phy-synopsys.c > drm/bridge/dw-hdmi/dw-hdmi-phy-*renesas*.c > drm/bridge/dw-hdmi/dw-hdmi-phy-something.c > Structures: > dw_hdmi_pdata > dw_hdmi_phy_pdata > dw_hdmi_phy_ops That looks good to me. > As the phy is interacted using controller we would need to pass > some callbacks to the phy, but ultimately the phy would be a > platform driver which could have its own compatible string that > would be associated with controller (not sure exactly about this > because I only used this in non-dt). We already have glue code, having to provide separate glue and PHY drivers seems a bit overkill to me. If we could get rid of glue code by adding a node for the PHY in DT that would be nice, but if we have to keep the glue then we can as well use platform data there. > This is just an idea though. I followed this logic in the RX side > and its very easy to add a new phy now, its a matter of creating > a new file, implement ops and associate it with controller. Of > course some phys will be very similar, for that we can use minor > tweaks via id detection. > > What do you think? It sounds neat overall, but I wonder whether it should really block this series, or be added on top of it :-) I think we're already moving in the right direction here. -- Regards, Laurent Pinchart
Re: [PATCH v2 2/3] v4l: Clearly document interactions between formats, controls and buffers
Hi Laurent, On Tue, Feb 28, 2017 at 05:03:19PM +0200, Laurent Pinchart wrote: > V4L2 exposes parameters that influence buffers sizes through the format > ioctls (VIDIOC_G_FMT, VIDIOC_TRY_FMT and VIDIO_S_FMT). Other parameters > not part of the format structure may also influence buffer sizes or > buffer layout in general. One existing such parameter is rotation, which > is implemented by the VIDIOC_ROTATE control and thus exposed through the > V4L2 control ioctls. > > The interaction between those parameters and buffers is currently only > partially specified by the V4L2 API. In particular interactions between > controls and buffers isn't specified at all. The behaviour of the > VIDIOC_S_FMT ioctl when buffers are allocated is also not fully > specified. > > This commit clearly defines and documents the interactions between > formats, controls and buffers. > > The preparatory discussions for the documentation change considered > completely disallowing controls that change the buffer size or layout, > in favour of extending the format API with a new ioctl that would bundle > those controls with format information. The idea has been rejected, as > this would essentially be a restricted version of the upcoming request > API that wouldn't bring any additional value. > > Another option we have considered was to mandate the use of the request > API to modify controls that influence buffer size or layout. This has > also been rejected on the grounds that requiring the request API to > change rotation even when streaming is stopped would significantly > complicate implementation of drivers and usage of the V4L2 API for > applications. > > Applications will however be required to use the upcoming request API to > change at runtime formats or controls that influence the buffer size or > layout, because of the need to synchronize buffers with the formats and > controls. Otherwise there would be no way to interpret the content of a > buffer correctly. > > Signed-off-by: Laurent Pinchart > --- > Documentation/media/uapi/v4l/buffer.rst | 88 > + > 1 file changed, 88 insertions(+) > > diff --git a/Documentation/media/uapi/v4l/buffer.rst > b/Documentation/media/uapi/v4l/buffer.rst > index ac58966ccb9b..5c58db98ab7a 100644 > --- a/Documentation/media/uapi/v4l/buffer.rst > +++ b/Documentation/media/uapi/v4l/buffer.rst > @@ -34,6 +34,94 @@ flags are copied from the OUTPUT video buffer to the > CAPTURE video > buffer. > > > +Interactions between formats, controls and buffers > +== > + > +V4L2 exposes parameters that influence the buffer size, or the way data is > +laid out in the buffer. Those parameters are exposed through both formats and > +controls. One example of such a control is the ``V4L2_CID_ROTATE`` control > +that modifies the direction in which pixels are stored in the buffer, as well > +as the buffer size when the selected format includes padding at the end of > +lines. > + > +The set of information needed to interpret the content of a buffer (e.g. the > +pixel format, the line stride, the tiling orientation or the rotation) is > +collectively referred to in the rest of this section as the buffer layout. > + > +Modifying formats or controls that influence the buffer size or layout > require > +the stream to be stopped. Any attempt at such a modification while the stream > +is active shall cause the format or control set ioctl to return the ``EBUSY`` > +error code. > + > +Controls that only influence the buffer layout can be modified at any time > +when the stream is stopped. As they don't influence the buffer size, no > +special handling is needed to synchronize those controls with buffer > +allocation. > + > +Formats and controls that influence the buffer size interact with buffer > +allocation. As buffer allocation is an expensive operation, drivers should > +allow format or controls that influence the buffer size to be changed with > +buffers allocated. A typical ioctl sequence to modify format and controls is > + > + #. VIDIOC_STREAMOFF > + #. VIDIOC_S_FMT > + #. VIDIOC_S_EXT_CTRLS Which one do you set first, the format or the controls? Supposedly the user would have to get the format again after setting the ROTATE control. > + #. VIDIOC_QBUF > + #. VIDIOC_STREAMON > + > +Queued buffers must be large enough for the new format or controls. > + > +Drivers shall return a ``ENOSPC`` error in response to format change > +(:c:func:`VIDIOC_S_FMT`) or control changes (:c:func:`VIDIOC_S_CTRL` or > +:c:func:`VIDIOC_S_EXT_CTRLS`) if buffers too small for the new format are > +currently queued. As a simplification, drivers are allowed to return an error > +from these ioctls if any buffer is currently queued, without checking the > +queued buffers sizes. Drivers shall also return a ``ENOSPC`` error from the > +:c:func:`VIDIOC_QBUF` ioctl if the buffer being queued is too small for the > +current format or controls. Togethe
Re: [PATCH v2 4/4] arm64: renesas: r8a7796: Enable HSCIF DMA
On Fri, Dec 09, 2016 at 10:58:25AM +0100, Geert Uytterhoeven wrote: > On Wed, Dec 7, 2016 at 5:44 PM, Ulrich Hecht > wrote: > > Signed-off-by: Ulrich Hecht > > Reviewed-by: Geert Uytterhoeven Thanks, I have queued this up for v4.12.
Re: [PATCH v4 0/2] arm64: dts: r8a7795: Add Cortex-A53 CPU cores and PMU
Hi Geert, On Fri, Feb 24, 2017 at 02:59:26PM +0100, Geert Uytterhoeven wrote: > Hi Simon, Magnus, > > This patch series adds the Cortex-A53 CPU cores and PMU on the Renesas > R-Car H3 SoC to its DTS file. > > Note that these patches describes the hardware; actual enabling of the > CPU depends on the PSCI firmware. > > With the current firmware version (v2.16.0), only the CA57 CPU cores are > enabled, hence these patches do not introduce undeterministic scheduling > behavior due to migration between big and LITTLE cores. > > Changes compared to v3: > - Drop bogus links from the pmu_a57 node to the CA53 nodes, > - Add Cortex-A53 PMU node. > > Tested on r8a7795/salvator-x, with CPU hot(un)plug and system suspend. > > Thanks for applying! I believe this patch-set is in keeping with the face-to-face discussion between Magnus, yourself, myself and others in Brussels last month. With that in mind I have queued this up for v4.12.
Re: [PATCH 0/3] arm64: dts: r8a7796: Add Secondary CPU Cores
On Fri, Feb 17, 2017 at 04:30:32PM +0100, Geert Uytterhoeven wrote: > Hi Simon, Magnus, > > This patch series adds the second Cortex-A57 CPU core, and the > Cortex-A53 L2 cache-controller and CPU nodes on the Renesas R-Car M3-W > SoC to its DTS file. > > Note that these patches add hardware description; actual enabling of the > CPU depends on the PSCI firmware. > > With the current firmware version (v2.16.0), only the CA57 CPU cores are > enabled, hence the last patch does not introduce undeterministic > scheduling behavior due to migration between big and LITTLE cores. > > Tested on r8a7796/salvator-x, with CPU hot(un)plug and system suspend. > > Thanks for applying! > > Geert Uytterhoeven (2): > arm64: dts: r8a7796: Add CA53 L2 cache-controller node > arm64: dts: r8a7796: Add Cortex-A53 CPU cores > > Takeshi Kihara (1): > arm64: dts: r8a7796: Add Cortex-A57 CPU cores Hi Geert, thanks for your work in this area. There seems to be some more work required to get patch 2/3 across the line so I am holding off on applying this series for now.
Re: [PATCH v4 1/3] watchdog: add rza_wdt driver
On 03/02/2017 05:57 AM, Chris Brandt wrote:
Adds a watchdog timer driver for the Renesas RZ/A Series SoCs. A reset
handler is also included since a WDT overflow is the only method for
restarting an RZ/A SoC.
Signed-off-by: Chris Brandt
---
drivers/watchdog/Kconfig | 8 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/rza_wdt.c | 208 +
3 files changed, 217 insertions(+)
create mode 100644 drivers/watchdog/rza_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index acb00b5..123c516 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -701,6 +701,14 @@ config RENESAS_WDT
This driver adds watchdog support for the integrated watchdogs in the
Renesas R-Car and other SH-Mobile SoCs (usually named RWDT or SWDT).
+config RENESAS_RZAWDT
+ tristate "Renesas RZ/A WDT Watchdog"
+ depends on ARCH_RENESAS || COMPILE_TEST
+ select WATCHDOG_CORE
+ help
+ This driver adds watchdog support for the integrated watchdogs in the
+ Renesas RZ/A SoCs. These watchdogs can be used to reset a system.
+
config ASPEED_WATCHDOG
tristate "Aspeed 2400 watchdog support"
depends on ARCH_ASPEED || COMPILE_TEST
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 0c3d35e..84b897c 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_LPC18XX_WATCHDOG) += lpc18xx_wdt.o
obj-$(CONFIG_BCM7038_WDT) += bcm7038_wdt.o
obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o
obj-$(CONFIG_RENESAS_WDT) += renesas_wdt.o
+obj-$(CONFIG_RENESAS_RZAWDT) += rza_wdt.o
obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o
# AVR32 Architecture
diff --git a/drivers/watchdog/rza_wdt.c b/drivers/watchdog/rza_wdt.c
new file mode 100644
index 000..17442c5
--- /dev/null
+++ b/drivers/watchdog/rza_wdt.c
@@ -0,0 +1,208 @@
+/*
+ * Renesas RZ/A Series WDT Driver
+ *
+ * Copyright (C) 2017 Renesas Electronics America, Inc.
+ * Copyright (C) 2017 Chris Brandt
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ *
The above two lines are unnecessary.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DEFAULT_TIMEOUT 30
+
+/* Watchdog Timer Registers */
+#define WTCSR 0
+#define WTCSR_MAGIC 0xA500
+#define WTSCR_WT (1<<6)
+#define WTSCR_TME (1<<5)
BIT()
+#define WTSCR_CKS(i) i
(i)
+
+#define WTCNT 2
+#define WTCNT_MAGIC 0x5A00
+
+#define WRCSR 4
+#define WRCSR_MAGIC 0x5A00
+#define WRCSR_RSTE (1<<6)
BIT()
+#define WRCSR_CLEAR_WOVF 0xA500 /* special value */
Please use
#define SOMETHINGvalue
throughout and make sure value is aligned.
+
+struct rza_wdt {
+ struct watchdog_device wdev;
+ void __iomem *base;
+ struct clk *clk;
+};
+
+static int rza_wdt_start(struct watchdog_device *wdev)
+{
+ struct rza_wdt *priv = watchdog_get_drvdata(wdev);
+
+ /* Stop timer */
+ writew(WTCSR_MAGIC | 0, priv->base + WTCSR);
+
+ /* Must dummy read WRCSR:WOVF at least once before clearing */
+ readb(priv->base + WRCSR);
+ writew(WRCSR_CLEAR_WOVF, priv->base + WRCSR);
+
+ /*
+* Start timer with slowest clock source and reset option enabled.
+*/
+ writew(WRCSR_MAGIC | WRCSR_RSTE, priv->base + WRCSR);
+ writew(WTCNT_MAGIC | 0, priv->base + WTCNT);
+ writew(WTCSR_MAGIC | WTSCR_WT | WTSCR_TME | WTSCR_CKS(7),
+ priv->base + WTCSR);
+
+ return 0;
+}
+
+static int rza_wdt_stop(struct watchdog_device *wdev)
+{
+ struct rza_wdt *priv = watchdog_get_drvdata(wdev);
+
+ writew(WTCSR_MAGIC | 0, priv->base + WTCSR);
+
+ return 0;
+}
+
+static int rza_wdt_ping(struct watchdog_device *wdev)
+{
+ struct rza_wdt *priv = watchdog_get_drvdata(wdev);
+
+ writew(WTCNT_MAGIC | 0, priv->base + WTCNT);
+
+ return 0;
+}
+
+static int rza_wdt_restart(struct watchdog_device *wdev, unsigned long action,
+ void *data)
+{
+ struct rza_wdt *priv = watchdog_get_drvdata(wdev);
+
+ /* Stop timer */
+ writew(WTCSR_MAGIC | 0, priv->base + WTCSR);
+
+ /* Must dummy read WRCSR:WOVF at least once before clearing */
+ readb(priv->base + WRCSR);
+ writew(WRCSR_CLEAR_WOVF, priv->base + WRCSR);
+
+ /*
+* Start timer with fastest clock source and only 1 clock left before
+* overflow with reset option enabled.
+*/
+ writew(WRCSR_MAGIC | WRCSR_RSTE, priv->base + WRCSR);
+ writew(WTCNT_MAGIC | 255, priv->base + WTCNT);
+ writew(WTCSR_MAGIC | WTSCR_WT | WTSCR_TME, priv->base + WTCSR);
+
+ /*
+* Actually make sure the above sequence hits hardware before sleeping.
+*/
+ wmb();
+
+ /* Wait for WDT overflow (reset) */
+ udelay(20);
+
+
Re: [PATCH v2 3/3] arm64: renesas: r8a7796: Enable SCIF DMA
On Tue, Feb 21, 2017 at 03:27:25PM +0100, Geert Uytterhoeven wrote: > Hi Simon, > > On Wed, Dec 7, 2016 at 5:44 PM, Ulrich Hecht > wrote: > > Signed-off-by: Ulrich Hecht > > --- > > arch/arm64/boot/dts/renesas/r8a7796.dtsi | 13 + > > 1 file changed, 13 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi > > b/arch/arm64/boot/dts/renesas/r8a7796.dtsi > > index c5f0df5..782063a 100644 > > --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi > > +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi > > @@ -430,6 +430,9 @@ > > <&cpg CPG_CORE R8A7796_CLK_S3D1>, > > <&scif_clk>; > > clock-names = "fck", "brg_int", "scif_clk"; > > + dmas = <&dmac1 0x51>, <&dmac1 0x50>, > > + <&dmac2 0x51>, <&dmac2 0x50>; > > + dma-names = "tx", "rx", "tx", "rx"; > > power-domains = <&sysc R8A7796_PD_ALWAYS_ON>; > > status = "disabled"; > > Apparently the DMA properties were added to the HSCIF nodes instead > of the SCIF nodes while applying this patch? > > 3 lines of context is not sufficient to distinguish between the various SCIF > and HSCIF nodes :-( Thanks, I have updated renesas-devel accordingly. I notice in renesas-devel there seems to be one "tx", "rx" DMA per SCIF rather than two added in Ulrich's patch. Is there a reason for that?
Re: [PATCH v4 7/9] drm: bridge: dw-hdmi: Add support for custom PHY configuration
Hi Laurent, On 02-03-2017 13:41, Laurent Pinchart wrote: > >> Hmm, this is kind of confusing. Why do you need a phy_ops and >> then a separate configure_phy function? Can't we just use phy_ops >> always? If its external phy then it would need to implement all >> phy_ops functions. > The phy_ops are indeed meant to support vendor PHYs. The .configure_phy() > operation is meant to support Synopsys PHYs that don't comply with the HDMI > TX > MHL and 3D PHYs I2C register layout and thus need a different configure > function, but can share the other operations with the HDMI TX MHL and 3D > PHYs. > Ideally I'd like to move that code to the dw-hdmi core, but at the moment I > don't have enough information to decide whether the register layout > corresponds to the standard Synopsys HDMI TX 2.0 PHY or if it has been > modified by the vendor. Once we'll have a second SoC using the same PHY we > should have more information to consolidate the code. Of course, if you can > share the HDMI TX 2.0 PHY datasheet, I won't mind reworking the code now ;-) > Well, if I want to keep my job I can't share the datasheet, and I do want to keep my job :) As far as I know this shouldn't change much. I already used (it was like a year ago) the dw-hdmi driver in a HDMI TX 2.0 PHY. But I am not following your logic here, sorry: You are mentioning a custom phy, right? If its custom then it should implement its own phy_ops. And I don't think that phy logic should go into core, this should all be extracted because I really believe it will make the code easier to read. Imagine this organization: Folders/Files: drm/bridge/dw-hdmi/dw-hdmi-core.c drm/bridge/dw-hdmi/dw-hdmi-phy-synopsys.c drm/bridge/dw-hdmi/dw-hdmi-phy-*renesas*.c drm/bridge/dw-hdmi/dw-hdmi-phy-something.c Structures: dw_hdmi_pdata dw_hdmi_phy_pdata dw_hdmi_phy_ops As the phy is interacted using controller we would need to pass some callbacks to the phy, but ultimately the phy would be a platform driver which could have its own compatible string that would be associated with controller (not sure exactly about this because I only used this in non-dt). This is just an idea though. I followed this logic in the RX side and its very easy to add a new phy now, its a matter of creating a new file, implement ops and associate it with controller. Of course some phys will be very similar, for that we can use minor tweaks via id detection. What do you think? Best regards, Jose Miguel Abreu
[PATCH v4 1/3] watchdog: add rza_wdt driver
Adds a watchdog timer driver for the Renesas RZ/A Series SoCs. A reset
handler is also included since a WDT overflow is the only method for
restarting an RZ/A SoC.
Signed-off-by: Chris Brandt
---
drivers/watchdog/Kconfig | 8 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/rza_wdt.c | 208 +
3 files changed, 217 insertions(+)
create mode 100644 drivers/watchdog/rza_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index acb00b5..123c516 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -701,6 +701,14 @@ config RENESAS_WDT
This driver adds watchdog support for the integrated watchdogs in the
Renesas R-Car and other SH-Mobile SoCs (usually named RWDT or SWDT).
+config RENESAS_RZAWDT
+ tristate "Renesas RZ/A WDT Watchdog"
+ depends on ARCH_RENESAS || COMPILE_TEST
+ select WATCHDOG_CORE
+ help
+ This driver adds watchdog support for the integrated watchdogs in the
+ Renesas RZ/A SoCs. These watchdogs can be used to reset a system.
+
config ASPEED_WATCHDOG
tristate "Aspeed 2400 watchdog support"
depends on ARCH_ASPEED || COMPILE_TEST
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 0c3d35e..84b897c 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_LPC18XX_WATCHDOG) += lpc18xx_wdt.o
obj-$(CONFIG_BCM7038_WDT) += bcm7038_wdt.o
obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o
obj-$(CONFIG_RENESAS_WDT) += renesas_wdt.o
+obj-$(CONFIG_RENESAS_RZAWDT) += rza_wdt.o
obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o
# AVR32 Architecture
diff --git a/drivers/watchdog/rza_wdt.c b/drivers/watchdog/rza_wdt.c
new file mode 100644
index 000..17442c5
--- /dev/null
+++ b/drivers/watchdog/rza_wdt.c
@@ -0,0 +1,208 @@
+/*
+ * Renesas RZ/A Series WDT Driver
+ *
+ * Copyright (C) 2017 Renesas Electronics America, Inc.
+ * Copyright (C) 2017 Chris Brandt
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DEFAULT_TIMEOUT 30
+
+/* Watchdog Timer Registers */
+#define WTCSR 0
+#define WTCSR_MAGIC 0xA500
+#define WTSCR_WT (1<<6)
+#define WTSCR_TME (1<<5)
+#define WTSCR_CKS(i) i
+
+#define WTCNT 2
+#define WTCNT_MAGIC 0x5A00
+
+#define WRCSR 4
+#define WRCSR_MAGIC 0x5A00
+#define WRCSR_RSTE (1<<6)
+#define WRCSR_CLEAR_WOVF 0xA500 /* special value */
+
+struct rza_wdt {
+ struct watchdog_device wdev;
+ void __iomem *base;
+ struct clk *clk;
+};
+
+static int rza_wdt_start(struct watchdog_device *wdev)
+{
+ struct rza_wdt *priv = watchdog_get_drvdata(wdev);
+
+ /* Stop timer */
+ writew(WTCSR_MAGIC | 0, priv->base + WTCSR);
+
+ /* Must dummy read WRCSR:WOVF at least once before clearing */
+ readb(priv->base + WRCSR);
+ writew(WRCSR_CLEAR_WOVF, priv->base + WRCSR);
+
+ /*
+* Start timer with slowest clock source and reset option enabled.
+*/
+ writew(WRCSR_MAGIC | WRCSR_RSTE, priv->base + WRCSR);
+ writew(WTCNT_MAGIC | 0, priv->base + WTCNT);
+ writew(WTCSR_MAGIC | WTSCR_WT | WTSCR_TME | WTSCR_CKS(7),
+ priv->base + WTCSR);
+
+ return 0;
+}
+
+static int rza_wdt_stop(struct watchdog_device *wdev)
+{
+ struct rza_wdt *priv = watchdog_get_drvdata(wdev);
+
+ writew(WTCSR_MAGIC | 0, priv->base + WTCSR);
+
+ return 0;
+}
+
+static int rza_wdt_ping(struct watchdog_device *wdev)
+{
+ struct rza_wdt *priv = watchdog_get_drvdata(wdev);
+
+ writew(WTCNT_MAGIC | 0, priv->base + WTCNT);
+
+ return 0;
+}
+
+static int rza_wdt_restart(struct watchdog_device *wdev, unsigned long action,
+ void *data)
+{
+ struct rza_wdt *priv = watchdog_get_drvdata(wdev);
+
+ /* Stop timer */
+ writew(WTCSR_MAGIC | 0, priv->base + WTCSR);
+
+ /* Must dummy read WRCSR:WOVF at least once before clearing */
+ readb(priv->base + WRCSR);
+ writew(WRCSR_CLEAR_WOVF, priv->base + WRCSR);
+
+ /*
+* Start timer with fastest clock source and only 1 clock left before
+* overflow with reset option enabled.
+*/
+ writew(WRCSR_MAGIC | WRCSR_RSTE, priv->base + WRCSR);
+ writew(WTCNT_MAGIC | 255, priv->base + WTCNT);
+ writew(WTCSR_MAGIC | WTSCR_WT | WTSCR_TME, priv->base + WTCSR);
+
+ /*
+* Actually make sure the above sequence hits hardware before sleeping.
+*/
+ wmb();
+
+ /* Wait for WDT overflow (reset) */
+ udelay(20);
+
+ return 0;
+}
+
+static const struct watchdog_info rza_wdt_ident = {
+ .options = WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
+ .identity = "Renesas RZ/A WDT Watchdo
[PATCH v4 3/3] ARM: dts: r7s72100: Add watchdog timer
Add watchdog timer support for RZ/A1.
For the RZ/A1, the only way to do a reset is to overflow the WDT, so this
is useful even if you don't need the watchdog functionality.
Signed-off-by: Chris Brandt
Reviewed-by: Geert Uytterhoeven
---
v4:
* changed from timer@ to watchdog@
v3:
* added Reviewed-by
v2:
* changed "renesas,r7s72100-reset" to "renesas,r7s72100-wdt"
* changed "renesas,wdt-reset" to "renesas,rza-wdt"
* added interupt property (even though it is not used)
* added clocks property
---
arch/arm/boot/dts/r7s72100.dtsi | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index ed62e19..22f96b7 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -382,6 +382,13 @@
cache-level = <2>;
};
+ wdt: watchdog@fcfe {
+ compatible = "renesas,r7s72100-wdt", "renesas,rza-wdt";
+ reg = <0xfcfe 0x6>;
+ interrupts = ;
+ clocks = <&p0_clk>;
+ };
+
i2c0: i2c@fcfee000 {
#address-cells = <1>;
#size-cells = <0>;
--
2.10.1
[PATCH v4 2/3] watchdog: renesas-wdt: add support for rza
Describe the WDT hardware in the RZ/A series. Signed-off-by: Chris Brandt Reviewed-by: Geert Uytterhoeven Acked-by: Rob Herring --- v3: * Add Acked-by, Reviewed-by. v2: * added to renesas-wdt.txt instead of creating a new file * changed commit title * added "renesas,rza-wdt" as a fallback * added interrupts property --- Documentation/devicetree/bindings/watchdog/renesas-wdt.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt b/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt index da24e31..9e306af 100644 --- a/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt +++ b/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt @@ -2,10 +2,11 @@ Renesas Watchdog Timer (WDT) Controller Required properties: - compatible : Should be "renesas,-wdt", and - "renesas,rcar-gen3-wdt" as fallback. + "renesas,rcar-gen3-wdt" or "renesas,rza-wdt" as fallback. Examples with soctypes are: - "renesas,r8a7795-wdt" (R-Car H3) - "renesas,r8a7796-wdt" (R-Car M3-W) +- "renesas,r7s72100-wdt" (RZ/A1) When compatible with the generic version, nodes must list the SoC-specific version corresponding to the platform first, followed by the generic @@ -17,6 +18,7 @@ Required properties: Optional properties: - timeout-sec : Contains the watchdog timeout in seconds - power-domains : the power domain the WDT belongs to +- interrupts: Some WDTs have an interrupt when used in interval timer mode Examples: -- 2.10.1
[PATCH v4 0/3] watchdog: add wdt and reset for renesas r7s72100
Some Renesas SoCs do not have a reset register and the only way to do a SW controlled reset is to use the watchdog timer. So while this series started out by only adding a reset feature, now it's a full watchdog timer driver that includes a reset handler. The longest WDT overflow you can get with a RZ/A1 (R7S72100) with its 8-bit wide counter is 125ms. Not very long. However, by setting max_hw_heartbeat_ms, the watchdog core will now handle pinging the WDT automatically and allow the user to set times as big as they want. Therefore, the default timeout for this driver is 30 seconds. Of course if system interrupts are disabled too long and wdt can't be pinged by the watchdog core, then the system will reset before the user specified timeout. But hey, it's better nothing. This driver was tested on an RZ/A1 RSK board using watchdog-simple.c from samples/watchdog/. v4: * r7s72100.dtsi: changed from timer@ to watchdog@ v3: * changed from a reset driver to a watchdog timer driver * use udelay(20) instead of msleep for reset handler * added Reviewed-by for r7s72100.dtsi and renesas-wdt.txt * added Acked-by for renesas-wdt.txt v2: * added to renesas-wdt.txt instead of creating a new file * changed "renesas,r7s72100-reset" to "renesas,r7s72100-wdt" * changed "renesas,wdt-reset" to "renesas,rza-wdt" * added "renesas,rza-wdt" as a fallback * added interupt property (even though it is not used) * added clocks property * changed hard coded register values to defines * added msleep to while(1) loop * removed unnecessary #include files * added Reviewed-by: Geert Uytterhoeven for renesas-reset.c Chris Brandt (3): watchdog: add rza_wdt driver watchdog: renesas-wdt: add support for rza ARM: dts: r7s72100: Add watchdog timer .../devicetree/bindings/watchdog/renesas-wdt.txt | 4 +- arch/arm/boot/dts/r7s72100.dtsi| 7 + drivers/watchdog/Kconfig | 8 + drivers/watchdog/Makefile | 1 + drivers/watchdog/rza_wdt.c | 208 + 5 files changed, 227 insertions(+), 1 deletion(-) create mode 100644 drivers/watchdog/rza_wdt.c -- 2.10.1
Re: [PATCH v4 7/9] drm: bridge: dw-hdmi: Add support for custom PHY configuration
Hi Jose,
On Thursday 02 Mar 2017 12:50:13 Jose Abreu wrote:
> On 01-03-2017 22:39, Laurent Pinchart wrote:
> > From: Kieran Bingham
> >
> > The DWC HDMI TX controller interfaces with a companion PHY. While
> > Synopsys provides multiple standard PHYs, SoC vendors can also integrate
> > a custom PHY.
> >
> > Modularize PHY configuration to support vendor PHYs through platform
> > data. The existing PHY configuration code was originally written to
> > support the DWC HDMI 3D TX PHY, and seems to be compatible with the DWC
> > MLP PHY. The HDMI 2.0 PHY will require a separate configuration
> > function.
> >
> > Signed-off-by: Kieran Bingham
> > Signed-off-by: Laurent Pinchart
> >
> > ---
> > Changes since v1:
> >
> > - Check pdata->phy_configure in hdmi_phy_configure() to avoid
> > dereferencing NULL pointer.
> >
> > ---
> >
> > drivers/gpu/drm/bridge/dw-hdmi.c | 109 ++
> > include/drm/bridge/dw_hdmi.h | 7 +++
> > 2 files changed, 81 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
> > b/drivers/gpu/drm/bridge/dw-hdmi.c index cb2703862be2..b835d81bb471
> > 100644
> > --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> > @@ -118,6 +118,9 @@ struct dw_hdmi_phy_data {
> > const char *name;
> > unsigned int gen;
> > bool has_svsret;
> > + int (*configure)(struct dw_hdmi *hdmi,
> > +const struct dw_hdmi_plat_data *pdata,
> > +unsigned long mpixelclock);
> > };
> >
> > struct dw_hdmi {
> > @@ -860,8 +863,8 @@ static bool hdmi_phy_wait_i2c_done(struct dw_hdmi
> > *hdmi, int msec)
> > return true;
> > }
> >
> > -static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
> > -unsigned char addr)
> > +void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
> > + unsigned char addr)
> > {
> > hdmi_writeb(hdmi, 0xFF, HDMI_IH_I2CMPHY_STAT0);
> > hdmi_writeb(hdmi, addr, HDMI_PHY_I2CM_ADDRESS_ADDR);
> > @@ -873,6 +876,7 @@ static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi,
> > unsigned short data,
> > HDMI_PHY_I2CM_OPERATION_ADDR);
> > hdmi_phy_wait_i2c_done(hdmi, 1000);
> > }
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_i2c_write);
> >
> > static void dw_hdmi_phy_enable_powerdown(struct dw_hdmi *hdmi, bool
> > enable)
> > {
> > @@ -993,37 +997,67 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi
> > *hdmi)
> > return 0;
> > }
> >
> > -static int hdmi_phy_configure(struct dw_hdmi *hdmi)
> > +/*
> > + * PHY configuration function for the DWC HDMI 3D TX PHY. Based on the
> > available
> > + * information the DWC MHL PHY has the same register layout and is thus
> > also
> > + * supported by this function.
> > + */
> > +static int hdmi_phy_configure_dwc_hdmi_3d_tx(struct dw_hdmi *hdmi,
> > + const struct dw_hdmi_plat_data *pdata,
> > + unsigned long mpixelclock)
> > {
> > - const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
> > - const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
> > const struct dw_hdmi_mpll_config *mpll_config = pdata->mpll_cfg;
> > const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
> > const struct dw_hdmi_phy_config *phy_config = pdata->phy_config;
> >
> > /* PLL/MPLL Cfg - always match on final entry */
> > for (; mpll_config->mpixelclock != ~0UL; mpll_config++)
> > - if (hdmi->hdmi_data.video_mode.mpixelclock <=
> > - mpll_config->mpixelclock)
> > + if (mpixelclock <= mpll_config->mpixelclock)
> > break;
> >
> > for (; curr_ctrl->mpixelclock != ~0UL; curr_ctrl++)
> > - if (hdmi->hdmi_data.video_mode.mpixelclock <=
> > - curr_ctrl->mpixelclock)
> > + if (mpixelclock <= curr_ctrl->mpixelclock)
> > break;
> >
> > for (; phy_config->mpixelclock != ~0UL; phy_config++)
> > - if (hdmi->hdmi_data.video_mode.mpixelclock <=
> > - phy_config->mpixelclock)
> > + if (mpixelclock <= phy_config->mpixelclock)
> > break;
> >
> > if (mpll_config->mpixelclock == ~0UL ||
> > curr_ctrl->mpixelclock == ~0UL ||
> > - phy_config->mpixelclock == ~0UL) {
> > - dev_err(hdmi->dev, "Pixel clock %d - unsupported by HDMI\n",
> > - hdmi->hdmi_data.video_mode.mpixelclock);
> > + phy_config->mpixelclock == ~0UL)
> > return -EINVAL;
> > - }
> > +
> > + dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[0].cpce,
> > + HDMI_3D_TX_PHY_CPCE_CTRL);
> > + dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[0].gmp,
> > + HDMI_3D_TX_PHY_GMPCTRL);
> > + dw_hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[0],
> > + HDMI_3D_TX_PHY_CURRCTRL);
> > +
> > + dw_hdmi_phy_i2c_write(hdmi, 0, HDMI_3
RE: [PATCH v3 3/3] ARM: dts: r7s72100: Add watchdog timer
Hello Sergei,
On Thursday, March 02, 2017, Sergei Shtylyov wrote:
> > diff --git a/arch/arm/boot/dts/r7s72100.dtsi
> > b/arch/arm/boot/dts/r7s72100.dtsi index ed62e19..6ecee72 100644
> > --- a/arch/arm/boot/dts/r7s72100.dtsi
> > +++ b/arch/arm/boot/dts/r7s72100.dtsi
> > @@ -382,6 +382,13 @@
> > cache-level = <2>;
> > };
> >
> > + wdt: timer@fcfe {
>
> Why not watchdog@...? That name is standardized in ePAPR...
>
You are right. I'll change that.
Thank you!
Chris
Re: [PATCH v4 7/9] drm: bridge: dw-hdmi: Add support for custom PHY configuration
Hi Laurent,
On 01-03-2017 22:39, Laurent Pinchart wrote:
> From: Kieran Bingham
>
> The DWC HDMI TX controller interfaces with a companion PHY. While
> Synopsys provides multiple standard PHYs, SoC vendors can also integrate
> a custom PHY.
>
> Modularize PHY configuration to support vendor PHYs through platform
> data. The existing PHY configuration code was originally written to
> support the DWC HDMI 3D TX PHY, and seems to be compatible with the DWC
> MLP PHY. The HDMI 2.0 PHY will require a separate configuration
> function.
>
> Signed-off-by: Kieran Bingham
> Signed-off-by: Laurent Pinchart
> ---
> Changes since v1:
>
> - Check pdata->phy_configure in hdmi_phy_configure() to avoid
> dereferencing NULL pointer.
> ---
> drivers/gpu/drm/bridge/dw-hdmi.c | 109
> ++-
> include/drm/bridge/dw_hdmi.h | 7 +++
> 2 files changed, 81 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index cb2703862be2..b835d81bb471 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -118,6 +118,9 @@ struct dw_hdmi_phy_data {
> const char *name;
> unsigned int gen;
> bool has_svsret;
> + int (*configure)(struct dw_hdmi *hdmi,
> + const struct dw_hdmi_plat_data *pdata,
> + unsigned long mpixelclock);
> };
>
> struct dw_hdmi {
> @@ -860,8 +863,8 @@ static bool hdmi_phy_wait_i2c_done(struct dw_hdmi *hdmi,
> int msec)
> return true;
> }
>
> -static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
> - unsigned char addr)
> +void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
> +unsigned char addr)
> {
> hdmi_writeb(hdmi, 0xFF, HDMI_IH_I2CMPHY_STAT0);
> hdmi_writeb(hdmi, addr, HDMI_PHY_I2CM_ADDRESS_ADDR);
> @@ -873,6 +876,7 @@ static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi,
> unsigned short data,
> HDMI_PHY_I2CM_OPERATION_ADDR);
> hdmi_phy_wait_i2c_done(hdmi, 1000);
> }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_i2c_write);
>
> static void dw_hdmi_phy_enable_powerdown(struct dw_hdmi *hdmi, bool enable)
> {
> @@ -993,37 +997,67 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
> return 0;
> }
>
> -static int hdmi_phy_configure(struct dw_hdmi *hdmi)
> +/*
> + * PHY configuration function for the DWC HDMI 3D TX PHY. Based on the
> available
> + * information the DWC MHL PHY has the same register layout and is thus also
> + * supported by this function.
> + */
> +static int hdmi_phy_configure_dwc_hdmi_3d_tx(struct dw_hdmi *hdmi,
> + const struct dw_hdmi_plat_data *pdata,
> + unsigned long mpixelclock)
> {
> - const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
> - const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
> const struct dw_hdmi_mpll_config *mpll_config = pdata->mpll_cfg;
> const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
> const struct dw_hdmi_phy_config *phy_config = pdata->phy_config;
>
> /* PLL/MPLL Cfg - always match on final entry */
> for (; mpll_config->mpixelclock != ~0UL; mpll_config++)
> - if (hdmi->hdmi_data.video_mode.mpixelclock <=
> - mpll_config->mpixelclock)
> + if (mpixelclock <= mpll_config->mpixelclock)
> break;
>
> for (; curr_ctrl->mpixelclock != ~0UL; curr_ctrl++)
> - if (hdmi->hdmi_data.video_mode.mpixelclock <=
> - curr_ctrl->mpixelclock)
> + if (mpixelclock <= curr_ctrl->mpixelclock)
> break;
>
> for (; phy_config->mpixelclock != ~0UL; phy_config++)
> - if (hdmi->hdmi_data.video_mode.mpixelclock <=
> - phy_config->mpixelclock)
> + if (mpixelclock <= phy_config->mpixelclock)
> break;
>
> if (mpll_config->mpixelclock == ~0UL ||
> curr_ctrl->mpixelclock == ~0UL ||
> - phy_config->mpixelclock == ~0UL) {
> - dev_err(hdmi->dev, "Pixel clock %d - unsupported by HDMI\n",
> - hdmi->hdmi_data.video_mode.mpixelclock);
> + phy_config->mpixelclock == ~0UL)
> return -EINVAL;
> - }
> +
> + dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[0].cpce,
> + HDMI_3D_TX_PHY_CPCE_CTRL);
> + dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[0].gmp,
> + HDMI_3D_TX_PHY_GMPCTRL);
> + dw_hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[0],
> + HDMI_3D_TX_PHY_CURRCTRL);
> +
> + dw_hdmi_phy_i2c_write(hdmi, 0, HDMI_3D_TX_PHY_PLLPHBYCTRL);
> + dw_hdmi_phy_i2c_write(hdmi, HDMI_3D_TX_PHY_MSM_CTRL_CKO_SEL_FB_CLK,
> + HDMI_3D_TX_PHY_MSM_CTRL);
> +
> + dw_hdmi_phy_i2c_write(hdmi, phy_config->t
Re: [PATCH v4 5/9] drm: bridge: dw-hdmi: Fix the PHY power up sequence
Hi Laurent,
On 01-03-2017 22:39, Laurent Pinchart wrote:
> When powering the PHY up we need to wait for the PLL to lock. This is
> done by polling the TX_PHY_LOCK bit in the HDMI_PHY_STAT0 register
> (interrupt-based wait could be implemented as well but is likely
> overkill). The bit is asserted when the PLL locks, but the current code
> incorrectly waits for the bit to be deasserted. Fix it, and while at it,
> replace the udelay() with a sleep as the code never runs in
> non-sleepable context.
>
> To be consistent with the power down implementation move the poll loop
> to the power off function.
>
> Signed-off-by: Laurent Pinchart
Reviewed-by: Jose Abreu
Best regards,
Jose Miguel Abreu
> ---
> drivers/gpu/drm/bridge/dw-hdmi.c | 65
> +++-
> 1 file changed, 37 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index 85348ba6bb1c..0aa3ad404f77 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -949,9 +949,44 @@ static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
> dw_hdmi_phy_gen2_pddq(hdmi, 1);
> }
>
> +static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
> +{
> + const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
> + unsigned int i;
> + u8 val;
> +
> + if (phy->gen == 1) {
> + dw_hdmi_phy_enable_powerdown(hdmi, false);
> +
> + /* Toggle TMDS enable. */
> + dw_hdmi_phy_enable_tmds(hdmi, 0);
> + dw_hdmi_phy_enable_tmds(hdmi, 1);
> + return 0;
> + }
> +
> + dw_hdmi_phy_gen2_txpwron(hdmi, 1);
> + dw_hdmi_phy_gen2_pddq(hdmi, 0);
> +
> + /* Wait for PHY PLL lock */
> + for (i = 0; i < 5; ++i) {
> + val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
> + if (val)
> + break;
> +
> + usleep_range(1000, 2000);
> + }
> +
> + if (!val) {
> + dev_err(hdmi->dev, "PHY PLL failed to lock\n");
> + return -ETIMEDOUT;
> + }
> +
> + dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
> + return 0;
> +}
> +
> static int hdmi_phy_configure(struct dw_hdmi *hdmi)
> {
> - u8 val, msec;
> const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
> const struct dw_hdmi_mpll_config *mpll_config = pdata->mpll_cfg;
> const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
> @@ -1019,33 +1054,7 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi)
> hdmi_phy_i2c_write(hdmi, HDMI_3D_TX_PHY_CKCALCTRL_OVERRIDE,
> HDMI_3D_TX_PHY_CKCALCTRL);
>
> - dw_hdmi_phy_enable_powerdown(hdmi, false);
> -
> - /* toggle TMDS enable */
> - dw_hdmi_phy_enable_tmds(hdmi, 0);
> - dw_hdmi_phy_enable_tmds(hdmi, 1);
> -
> - /* gen2 tx power on */
> - dw_hdmi_phy_gen2_txpwron(hdmi, 1);
> - dw_hdmi_phy_gen2_pddq(hdmi, 0);
> -
> - /* Wait for PHY PLL lock */
> - msec = 5;
> - do {
> - val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
> - if (!val)
> - break;
> -
> - if (msec == 0) {
> - dev_err(hdmi->dev, "PHY PLL not locked\n");
> - return -ETIMEDOUT;
> - }
> -
> - udelay(1000);
> - msec--;
> - } while (1);
> -
> - return 0;
> + return dw_hdmi_phy_power_on(hdmi);
> }
>
> static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
Re: [PATCH v4 6/9] drm: bridge: dw-hdmi: Create PHY operations
Hi Laurent,
On 01-03-2017 22:39, Laurent Pinchart wrote:
> The HDMI TX controller support different PHYs whose programming
> interface can vary significantly, especially with vendor PHYs that are
> not provided by Synopsys. To support them, create a PHY operation
> structure that can be provided by the platform glue layer. The existing
> PHY handling code (limited to Synopsys PHY support) is refactored into a
> set of default PHY operations that are used automatically when the
> platform glue doesn't provide its own operations.
>
> Signed-off-by: Laurent Pinchart
Reviewed-by: Jose Abreu
Best regards,
Jose Miguel Abreu
> ---
> drivers/gpu/drm/bridge/dw-hdmi.c | 91
>
> include/drm/bridge/dw_hdmi.h | 18 +++-
> 2 files changed, 80 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index 0aa3ad404f77..cb2703862be2 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -141,8 +141,12 @@ struct dw_hdmi {
> u8 edid[HDMI_EDID_LEN];
> bool cable_plugin;
>
> - const struct dw_hdmi_phy_data *phy;
> - bool phy_enabled;
> + struct {
> + const struct dw_hdmi_phy_ops *ops;
> + const char *name;
> + void *data;
> + bool enabled;
> + } phy;
>
> struct drm_display_mode previous_mode;
>
> @@ -831,6 +835,10 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi)
> HDMI_VP_CONF);
> }
>
> +/*
> -
> + * Synopsys PHY Handling
> + */
> +
> static inline void hdmi_phy_test_clear(struct dw_hdmi *hdmi,
> unsigned char bit)
> {
> @@ -987,6 +995,7 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
>
> static int hdmi_phy_configure(struct dw_hdmi *hdmi)
> {
> + const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
> const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
> const struct dw_hdmi_mpll_config *mpll_config = pdata->mpll_cfg;
> const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
> @@ -1019,7 +1028,7 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi)
> dw_hdmi_phy_power_off(hdmi);
>
> /* Leave low power consumption mode by asserting SVSRET. */
> - if (hdmi->phy->has_svsret)
> + if (phy->has_svsret)
> dw_hdmi_phy_enable_svsret(hdmi, 1);
>
> /* PHY reset. The reset signal is active high on Gen2 PHYs. */
> @@ -1057,7 +1066,8 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi)
> return dw_hdmi_phy_power_on(hdmi);
> }
>
> -static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
> +static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data,
> + struct drm_display_mode *mode)
> {
> int i, ret;
>
> @@ -1071,10 +1081,31 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
> return ret;
> }
>
> - hdmi->phy_enabled = true;
> return 0;
> }
>
> +static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi, void *data)
> +{
> + dw_hdmi_phy_power_off(hdmi);
> +}
> +
> +static enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
> + void *data)
> +{
> + return hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_HPD ?
> + connector_status_connected : connector_status_disconnected;
> +}
> +
> +static const struct dw_hdmi_phy_ops dw_hdmi_synopsys_phy_ops = {
> + .init = dw_hdmi_phy_init,
> + .disable = dw_hdmi_phy_disable,
> + .read_hpd = dw_hdmi_phy_read_hpd,
> +};
> +
> +/*
> -
> + * HDMI TX Setup
> + */
> +
> static void hdmi_tx_hdcp_config(struct dw_hdmi *hdmi)
> {
> u8 de;
> @@ -1289,16 +1320,6 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
> hdmi_writeb(hdmi, vsync_len, HDMI_FC_VSYNCINWIDTH);
> }
>
> -static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi)
> -{
> - if (!hdmi->phy_enabled)
> - return;
> -
> - dw_hdmi_phy_power_off(hdmi);
> -
> - hdmi->phy_enabled = false;
> -}
> -
> /* HDMI Initialization Step B.4 */
> static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi)
> {
> @@ -1431,9 +1452,10 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct
> drm_display_mode *mode)
> hdmi_av_composer(hdmi, mode);
>
> /* HDMI Initializateion Step B.2 */
> - ret = dw_hdmi_phy_init(hdmi);
> + ret = hdmi->phy.ops->init(hdmi, hdmi->phy.data, &hdmi->previous_mode);
> if (ret)
> return ret;
> + hdmi->phy.enabled = true;
>
> /* HDMI Initialization Step B.3 */
> dw_hdmi_enable_video_path(hdmi);
> @@ -1548,7 +1570,11 @@ static void dw_hdmi_poweron(struct dw_hdmi *hdmi)
>
> static void dw_hdmi_poweroff(struct dw_hdmi *hdmi)
>
Re: [PATCH v4 4/9] drm: bridge: dw-hdmi: Fix the PHY power down sequence
Hi Laurent,
On 01-03-2017 22:39, Laurent Pinchart wrote:
> The PHY requires us to wait for the PHY to switch to low power mode
> after deasserting TXPWRON and before asserting PDDQ in the power down
> sequence, otherwise power down will fail.
>
> The PHY power down can be monitored though the TX_READY bit, available
> through I2C in the PHY registers, or the TX_PHY_LOCK bit, available
> through the HDMI TX registers. As the two are equivalent, let's pick the
> easier solution of polling the TX_PHY_LOCK bit.
>
> The power down code is currently duplicated in multiple places. To avoid
> spreading multiple calls to a TX_PHY_LOCK poll function, we have to
> refactor the power down code and group it all in a single function.
>
> Tests showed that one poll iteration was enough for TX_PHY_LOCK to
> become low, without requiring any additional delay. Retrying the read
> five times with a 1ms to 2ms delay between each attempt should thus be
> more than enough.
>
> Signed-off-by: Laurent Pinchart
Reviewed-by: Jose Abreu
Best regards,
Jose Miguel Abreu
> ---
> drivers/gpu/drm/bridge/dw-hdmi.c | 52
> +---
> 1 file changed, 43 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index d863b3393aee..85348ba6bb1c 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -116,6 +116,7 @@ struct dw_hdmi_i2c {
> struct dw_hdmi_phy_data {
> enum dw_hdmi_phy_type type;
> const char *name;
> + unsigned int gen;
> bool has_svsret;
> };
>
> @@ -914,6 +915,40 @@ static void dw_hdmi_phy_sel_interface_control(struct
> dw_hdmi *hdmi, u8 enable)
>HDMI_PHY_CONF0_SELDIPIF_MASK);
> }
>
> +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
> +{
> + const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
> + unsigned int i;
> + u16 val;
> +
> + if (phy->gen == 1) {
> + dw_hdmi_phy_enable_tmds(hdmi, 0);
> + dw_hdmi_phy_enable_powerdown(hdmi, true);
> + return;
> + }
> +
> + dw_hdmi_phy_gen2_txpwron(hdmi, 0);
> +
> + /*
> + * Wait for TX_PHY_LOCK to be deasserted to indicate that the PHY went
> + * to low power mode.
> + */
> + for (i = 0; i < 5; ++i) {
> + val = hdmi_readb(hdmi, HDMI_PHY_STAT0);
> + if (!(val & HDMI_PHY_TX_PHY_LOCK))
> + break;
> +
> + usleep_range(1000, 2000);
> + }
> +
> + if (val & HDMI_PHY_TX_PHY_LOCK)
> + dev_warn(hdmi->dev, "PHY failed to power down\n");
> + else
> + dev_dbg(hdmi->dev, "PHY powered down in %u iterations\n", i);
> +
> + dw_hdmi_phy_gen2_pddq(hdmi, 1);
> +}
> +
> static int hdmi_phy_configure(struct dw_hdmi *hdmi)
> {
> u8 val, msec;
> @@ -946,11 +981,7 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi)
> return -EINVAL;
> }
>
> - /* gen2 tx power off */
> - dw_hdmi_phy_gen2_txpwron(hdmi, 0);
> -
> - /* gen2 pddq */
> - dw_hdmi_phy_gen2_pddq(hdmi, 1);
> + dw_hdmi_phy_power_off(hdmi);
>
> /* Leave low power consumption mode by asserting SVSRET. */
> if (hdmi->phy->has_svsret)
> @@ -1025,8 +1056,6 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
> for (i = 0; i < 2; i++) {
> dw_hdmi_phy_sel_data_en_pol(hdmi, 1);
> dw_hdmi_phy_sel_interface_control(hdmi, 0);
> - dw_hdmi_phy_enable_tmds(hdmi, 0);
> - dw_hdmi_phy_enable_powerdown(hdmi, true);
>
> ret = hdmi_phy_configure(hdmi);
> if (ret)
> @@ -1256,8 +1285,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi)
> if (!hdmi->phy_enabled)
> return;
>
> - dw_hdmi_phy_enable_tmds(hdmi, 0);
> - dw_hdmi_phy_enable_powerdown(hdmi, true);
> + dw_hdmi_phy_power_off(hdmi);
>
> hdmi->phy_enabled = false;
> }
> @@ -1827,23 +1855,29 @@ static const struct dw_hdmi_phy_data dw_hdmi_phys[] =
> {
> {
> .type = DW_HDMI_PHY_DWC_HDMI_TX_PHY,
> .name = "DWC HDMI TX PHY",
> + .gen = 1,
> }, {
> .type = DW_HDMI_PHY_DWC_MHL_PHY_HEAC,
> .name = "DWC MHL PHY + HEAC PHY",
> + .gen = 2,
> .has_svsret = true,
> }, {
> .type = DW_HDMI_PHY_DWC_MHL_PHY,
> .name = "DWC MHL PHY",
> + .gen = 2,
> .has_svsret = true,
> }, {
> .type = DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY_HEAC,
> .name = "DWC HDMI 3D TX PHY + HEAC PHY",
> + .gen = 2,
> }, {
> .type = DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY,
> .name = "DWC HDMI 3D TX PHY",
> + .gen = 2,
> }, {
> .type = DW_HDMI_PHY_DWC_HDMI20_TX_PHY,
> .name = "DWC HDMI 2.0 TX PHY",
> + .gen = 2,
>
Re: [PATCH v4 0/9] drm: bridge: dw-hdmi: Refactor PHY support
On 03/01/2017 11:39 PM, Laurent Pinchart wrote: > Hello, > > This patch series refactors all the PHY handling code in order to allow > support of vendor PHYs and Synopsys DWC HDMI 2.0 TX PHYs. > > The series starts with a few cleanups and small fixes. Patch 1/9 just removes > unused code, patch 2/9 moves the color converter code out of the PHY configure > function as it isn't PHY-dependent, and patch 3/9 enables color conversion > even for DVI as it is needed to output RGB when the input format is YUV. > > The next two patches fix the power down (4/9) and up (5/9) sequences to comply > with the HDMI TX PHY specifications. They are the biggest functional changes > in the whole set, and have been tested successfully (with the rest of the > series) on i.MX6Q and R-Car H3. I'll try to perform tests on RK3288 tomorrow > if nobody beats me to it (Neil, that's for you :-)). Done ! Tested on RK3288 on ACT8846 EVB Board and Amlogic S905X P230 Board. Tested-by: Neil Armstrong Thanks, Neil > The PLL PHY lock delay > has been measured to be between 300µs and 350µs on R-Car H3 and between 400µs > and 600µs on i.MX6Q. The PHY power down delay has been measured to be less > than 50µs on both platforms, and was often close to instant with power down > reported in the first poll iteration. We should thus be more than safe with a > 5ms timeout. > > Patch 6/9 breaks the PHY operations out. Glue code is then allowed to pass a > PHY operations structure to support vendor PHYs. The existing PHY support code > is turned into a default Synopsys PHYs implementation for those PHY > operations. > > Patch 7/9 further refactors the Synopsys PHY configuration function to make > it modular, in order to support DWC HDMI 2.0 TX PHYs that have a very > different register layout compared to the currently supported PHYs. Glue code > is again allowed to provide a custom PHY configuration implementation, with > the existing PHY support code turned into the default implementation for all > currently supported Synopsys PHYs. > > Patch 8/9 is a small cleanup that removes the now unneeded device type for > glue code platform data, and patch 9/9 follows by switching the driver to > regmap in order to support vendor-specific register access more easily. > > Kieran Bingham (2): > drm: bridge: dw-hdmi: Add support for custom PHY configuration > drm: bridge: dw-hdmi: Remove device type from platform data > > Laurent Pinchart (5): > drm: bridge: dw-hdmi: Remove unused functions > drm: bridge: dw-hdmi: Move CSC configuration out of PHY code > drm: bridge: dw-hdmi: Fix the PHY power down sequence > drm: bridge: dw-hdmi: Fix the PHY power up sequence > drm: bridge: dw-hdmi: Create PHY operations > > Neil Armstrong (2): > drm: bridge: dw-hdmi: Enable CSC even for DVI > drm: bridge: dw-hdmi: Switch to regmap for register access > > drivers/gpu/drm/bridge/dw-hdmi.c| 467 > +--- > drivers/gpu/drm/imx/dw_hdmi-imx.c | 2 - > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 1 - > include/drm/bridge/dw_hdmi.h| 33 +- > 4 files changed, 304 insertions(+), 199 deletions(-) >
Re: [PATCH v4 0/9] drm: bridge: dw-hdmi: Refactor PHY support
Hi Neil, On Thursday 02 Mar 2017 12:27:52 Neil Armstrong wrote: > On 03/01/2017 11:39 PM, Laurent Pinchart wrote: > > Hello, > > > > This patch series refactors all the PHY handling code in order to allow > > support of vendor PHYs and Synopsys DWC HDMI 2.0 TX PHYs. > > > > The series starts with a few cleanups and small fixes. Patch 1/9 just > > removes unused code, patch 2/9 moves the color converter code out of the > > PHY configure function as it isn't PHY-dependent, and patch 3/9 enables > > color conversion even for DVI as it is needed to output RGB when the > > input format is YUV. > > > > The next two patches fix the power down (4/9) and up (5/9) sequences to > > comply with the HDMI TX PHY specifications. They are the biggest > > functional changes in the whole set, and have been tested successfully > > (with the rest of the series) on i.MX6Q and R-Car H3. I'll try to perform > > tests on RK3288 tomorrow if nobody beats me to it (Neil, that's for you > > :-)). > > Done ! > > Tested on RK3288 on ACT8846 EVB Board and Amlogic S905X P230 Board. > > Tested-by: Neil Armstrong Thank you ! I manage to test it on a remote RK3288 board too today, and the PLL lock delay was between 150µs and 450µs. > > The PLL PHY lock delay has been measured to be between 300µs and 350µs on > > R-Car H3 and between 400µs and 600µs on i.MX6Q. The PHY power down delay > > has been measured to be less than 50µs on both platforms, and was often > > close to instant with power down reported in the first poll iteration. We > > should thus be more than safe with a 5ms timeout. > > > > Patch 6/9 breaks the PHY operations out. Glue code is then allowed to pass > > a PHY operations structure to support vendor PHYs. The existing PHY > > support code is turned into a default Synopsys PHYs implementation for > > those PHY operations. > > > > Patch 7/9 further refactors the Synopsys PHY configuration function to > > make > > it modular, in order to support DWC HDMI 2.0 TX PHYs that have a very > > different register layout compared to the currently supported PHYs. Glue > > code is again allowed to provide a custom PHY configuration > > implementation, with the existing PHY support code turned into the > > default implementation for all currently supported Synopsys PHYs. > > > > Patch 8/9 is a small cleanup that removes the now unneeded device type for > > glue code platform data, and patch 9/9 follows by switching the driver to > > regmap in order to support vendor-specific register access more easily. > > > > Kieran Bingham (2): > > drm: bridge: dw-hdmi: Add support for custom PHY configuration > > drm: bridge: dw-hdmi: Remove device type from platform data > > > > Laurent Pinchart (5): > > drm: bridge: dw-hdmi: Remove unused functions > > drm: bridge: dw-hdmi: Move CSC configuration out of PHY code > > drm: bridge: dw-hdmi: Fix the PHY power down sequence > > drm: bridge: dw-hdmi: Fix the PHY power up sequence > > drm: bridge: dw-hdmi: Create PHY operations > > > > Neil Armstrong (2): > > drm: bridge: dw-hdmi: Enable CSC even for DVI > > drm: bridge: dw-hdmi: Switch to regmap for register access > > > > drivers/gpu/drm/bridge/dw-hdmi.c| 467 +- > > drivers/gpu/drm/imx/dw_hdmi-imx.c | 2 - > > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 1 - > > include/drm/bridge/dw_hdmi.h| 33 +- > > 4 files changed, 304 insertions(+), 199 deletions(-) -- Regards, Laurent Pinchart
Re: [PATCH v4 1/9] drm: bridge: dw-hdmi: Remove unused functions
Hi Laurent,
On 01-03-2017 22:39, Laurent Pinchart wrote:
> Most of the hdmi_phy_test_*() functions are unused. Remove them.
>
> Signed-off-by: Laurent Pinchart
Reviewed-by: Jose Abreu
Best regards,
Jose Miguel Abreu
> ---
> drivers/gpu/drm/bridge/dw-hdmi.c | 26 --
> 1 file changed, 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index 9a9ec27d9e28..ce7496399ccf 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -837,32 +837,6 @@ static inline void hdmi_phy_test_clear(struct dw_hdmi
> *hdmi,
> HDMI_PHY_TST0_TSTCLR_MASK, HDMI_PHY_TST0);
> }
>
> -static inline void hdmi_phy_test_enable(struct dw_hdmi *hdmi,
> - unsigned char bit)
> -{
> - hdmi_modb(hdmi, bit << HDMI_PHY_TST0_TSTEN_OFFSET,
> - HDMI_PHY_TST0_TSTEN_MASK, HDMI_PHY_TST0);
> -}
> -
> -static inline void hdmi_phy_test_clock(struct dw_hdmi *hdmi,
> -unsigned char bit)
> -{
> - hdmi_modb(hdmi, bit << HDMI_PHY_TST0_TSTCLK_OFFSET,
> - HDMI_PHY_TST0_TSTCLK_MASK, HDMI_PHY_TST0);
> -}
> -
> -static inline void hdmi_phy_test_din(struct dw_hdmi *hdmi,
> - unsigned char bit)
> -{
> - hdmi_writeb(hdmi, bit, HDMI_PHY_TST1);
> -}
> -
> -static inline void hdmi_phy_test_dout(struct dw_hdmi *hdmi,
> - unsigned char bit)
> -{
> - hdmi_writeb(hdmi, bit, HDMI_PHY_TST2);
> -}
> -
> static bool hdmi_phy_wait_i2c_done(struct dw_hdmi *hdmi, int msec)
> {
> u32 val;
Re: [PATCH v4 2/9] drm: bridge: dw-hdmi: Move CSC configuration out of PHY code
Hi Laurent,
On 01-03-2017 22:39, Laurent Pinchart wrote:
> The color space converter isn't part of the PHY, move its configuration
> out of PHY code.
>
> Signed-off-by: Laurent Pinchart
Reviewed-by: Jose Abreu
Best regards,
Jose Miguel Abreu
> ---
> drivers/gpu/drm/bridge/dw-hdmi.c | 25 ++---
> 1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index ce7496399ccf..906583beb08b 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -914,7 +914,7 @@ static void dw_hdmi_phy_sel_interface_control(struct
> dw_hdmi *hdmi, u8 enable)
>HDMI_PHY_CONF0_SELDIPIF_MASK);
> }
>
> -static int hdmi_phy_configure(struct dw_hdmi *hdmi, int cscon)
> +static int hdmi_phy_configure(struct dw_hdmi *hdmi)
> {
> u8 val, msec;
> const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
> @@ -946,14 +946,6 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, int
> cscon)
> return -EINVAL;
> }
>
> - /* Enable csc path */
> - if (cscon)
> - val = HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_IN_PATH;
> - else
> - val = HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_BYPASS;
> -
> - hdmi_writeb(hdmi, val, HDMI_MC_FLOWCTRL);
> -
> /* gen2 tx power off */
> dw_hdmi_phy_gen2_txpwron(hdmi, 0);
>
> @@ -1028,10 +1020,6 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi,
> int cscon)
> static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
> {
> int i, ret;
> - bool cscon;
> -
> - /*check csc whether needed activated in HDMI mode */
> - cscon = hdmi->sink_is_hdmi && is_color_space_conversion(hdmi);
>
> /* HDMI Phy spec says to do the phy initialization sequence twice */
> for (i = 0; i < 2; i++) {
> @@ -1040,8 +1028,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
> dw_hdmi_phy_enable_tmds(hdmi, 0);
> dw_hdmi_phy_enable_powerdown(hdmi, true);
>
> - /* Enable CSC */
> - ret = hdmi_phy_configure(hdmi, cscon);
> + ret = hdmi_phy_configure(hdmi);
> if (ret)
> return ret;
> }
> @@ -1303,6 +1290,14 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi
> *hdmi)
> clkdis &= ~HDMI_MC_CLKDIS_CSCCLK_DISABLE;
> hdmi_writeb(hdmi, clkdis, HDMI_MC_CLKDIS);
> }
> +
> + /* Enable color space conversion if needed (for HDMI sinks only). */
> + if (hdmi->sink_is_hdmi && is_color_space_conversion(hdmi))
> + hdmi_writeb(hdmi, HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_IN_PATH,
> + HDMI_MC_FLOWCTRL);
> + else
> + hdmi_writeb(hdmi, HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_BYPASS,
> + HDMI_MC_FLOWCTRL);
> }
>
> static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi)
[PATCH v4 0/4] drm: bridge: New LVDS encoder driver
Hello, This series is a rebase of the LVDS encoder driver previously posted as part of "[PATCH v3 00/13] R-Car DU: Use drm bridge API" on top of the latest drm-misc. The DT bindings have been acked by Rob Herring, and the vga-dac change by Maxime Ripard. Daniel Vetter reported in a public reply to "[PATCH v3 06/13] drm: bridge: Add LVDS encoder driver" that he was fine with the patch and gave me permission to add his Acked-by in a private conversation. For convenience the patches are available in a git branch at git://linuxtv.org/pinchartl/media.git drm-next-lvds-encoder-v4-20170302 Archit, would you be able to do a final review of the driver code and merge it if everything looks good to you ? Laurent Pinchart (4): devicetree/bindings: display: bridge: Add LVDS encoder DT bindings drm: bridge: Add LVDS encoder driver drm: bridge: vga-dac: Add adi,adv7123 compatible string drm: bridge: lvds-encoder: Add thine,thc63lvdm83d compatible string .../bindings/display/bridge/lvds-transmitter.txt | 64 +++ drivers/gpu/drm/bridge/Kconfig | 9 + drivers/gpu/drm/bridge/Makefile| 1 + drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 + drivers/gpu/drm/bridge/lvds-encoder.c | 210 + 5 files changed, 285 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt create mode 100644 drivers/gpu/drm/bridge/lvds-encoder.c -- Regards, Laurent Pinchart
[PATCH v4 3/4] drm: bridge: vga-dac: Add adi,adv7123 compatible string
The ADV7123 is a transparent VGA DAC. Unlike dumb VGA DACs it can be
controlled through a power save pin, and requires a power supply.
However, on most boards where the device is used neither the power save
signal nor the power supply are controllable.
To avoid developing a separate device-specific driver add an
"adi,adv7123" compatible entry to the dumb-vga-dac driver. This will
allow supporting most ADV7123-based boards easily, while allowing future
development of an adv7123 driver when needed without breaking backward
compatibility.
Signed-off-by: Laurent Pinchart
Acked-by: Maxime Ripard
---
drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index 86e9f9c7b59c..63e113bd21d2 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -237,6 +237,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
static const struct of_device_id dumb_vga_match[] = {
{ .compatible = "dumb-vga-dac" },
+ { .compatible = "adi,adv7123" },
{ .compatible = "ti,ths8135" },
{},
};
--
Regards,
Laurent Pinchart
[PATCH v4 2/4] drm: bridge: Add LVDS encoder driver
The LVDS encoder driver is a DRM bridge driver that supports the
parallel to LVDS encoders that don't require any configuration. The
driver thus doesn't interact with the device, but creates an LVDS
connector for the panel and exposes its size and timing based on
information retrieved from DT.
Signed-off-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
Changes since v3:
- Dropped encoder_type
- Added a Kconfig dependency on DRM_PANEL
---
drivers/gpu/drm/bridge/Kconfig| 9 ++
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/lvds-encoder.c | 209 ++
3 files changed, 219 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/lvds-encoder.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index eb8688ec6f18..0c62d6ee1c88 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -24,6 +24,15 @@ config DRM_DUMB_VGA_DAC
help
Support for RGB to VGA DAC based bridges
+config DRM_LVDS_ENCODER
+ tristate "Transparent parallel to LVDS encoder support"
+ depends on OF
+ select DRM_KMS_HELPER
+ select DRM_PANEL
+ help
+ Support for transparent parallel to LVDS encoders that don't require
+ any configuration.
+
config DRM_DW_HDMI
tristate
select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 2e83a7855399..f675ecabb609 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -2,6 +2,7 @@ ccflags-y := -Iinclude/drm
obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
+obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
obj-$(CONFIG_DRM_DW_HDMI_I2S_AUDIO) += dw-hdmi-i2s-audio.o
diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c
b/drivers/gpu/drm/bridge/lvds-encoder.c
new file mode 100644
index ..d817d78b893d
--- /dev/null
+++ b/drivers/gpu/drm/bridge/lvds-encoder.c
@@ -0,0 +1,209 @@
+/*
+ * Copyright (C) 2016 Laurent Pinchart
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+struct lvds_encoder {
+ struct device *dev;
+
+ struct drm_bridge bridge;
+ struct drm_connector connector;
+ struct drm_panel *panel;
+};
+
+static inline struct lvds_encoder *
+drm_bridge_to_lvds_encoder(struct drm_bridge *bridge)
+{
+ return container_of(bridge, struct lvds_encoder, bridge);
+}
+
+static inline struct lvds_encoder *
+drm_connector_to_lvds_encoder(struct drm_connector *connector)
+{
+ return container_of(connector, struct lvds_encoder, connector);
+}
+
+static int lvds_connector_get_modes(struct drm_connector *connector)
+{
+ struct lvds_encoder *lvds = drm_connector_to_lvds_encoder(connector);
+
+ return drm_panel_get_modes(lvds->panel);
+}
+
+static const struct drm_connector_helper_funcs lvds_connector_helper_funcs = {
+ .get_modes = lvds_connector_get_modes,
+};
+
+static const struct drm_connector_funcs lvds_connector_funcs = {
+ .dpms = drm_atomic_helper_connector_dpms,
+ .reset = drm_atomic_helper_connector_reset,
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .destroy = drm_connector_cleanup,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int lvds_encoder_attach(struct drm_bridge *bridge)
+{
+ struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
+ struct drm_connector *connector = &lvds->connector;
+ int ret;
+
+ if (!bridge->encoder) {
+ DRM_ERROR("Missing encoder\n");
+ return -ENODEV;
+ }
+
+ drm_connector_helper_add(connector, &lvds_connector_helper_funcs);
+
+ ret = drm_connector_init(bridge->dev, connector, &lvds_connector_funcs,
+DRM_MODE_CONNECTOR_LVDS);
+ if (ret) {
+ DRM_ERROR("Failed to initialize connector\n");
+ return ret;
+ }
+
+ drm_mode_connector_attach_encoder(&lvds->connector, bridge->encoder);
+
+ ret = drm_panel_attach(lvds->panel, &lvds->connector);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static void lvds_encoder_detach(struct drm_bridge *bridge)
+{
+ struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
+
+ drm_panel_detach(lvds->panel);
+}
+
+static void lvds_encoder_pre_enable(struct drm_bridge *bridge)
+{
+ struct lv
[PATCH v4 4/4] drm: bridge: lvds-encoder: Add thine,thc63lvdm83d compatible string
The THC63LVDM83D is a transparent LVDS encoder. Unlike dumb LVDS
encoders it can be controlled through a few pins (power down, LVDS
swing, clock edge selection) and requires power supplies. However, on
several boards where the device is used neither the control pins nor the
power supply are controllable.
To avoid developing a separate device-specific driver add a
"thine,thc63lvdm83d" compatible entry to the lvds-encoder driver. This
will allow supporting many THC63LVDM83D-based boards easily, while
allowing future development of an thc63lvdm83d driver when needed
without breaking backward compatibility.
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/bridge/lvds-encoder.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c
b/drivers/gpu/drm/bridge/lvds-encoder.c
index d817d78b893d..6ec2aafcfe85 100644
--- a/drivers/gpu/drm/bridge/lvds-encoder.c
+++ b/drivers/gpu/drm/bridge/lvds-encoder.c
@@ -190,6 +190,7 @@ static int lvds_encoder_remove(struct platform_device *pdev)
static const struct of_device_id lvds_encoder_match[] = {
{ .compatible = "lvds-encoder" },
+ { .compatible = "thine,thc63lvdm83d" },
{},
};
MODULE_DEVICE_TABLE(of, lvds_encoder_match);
--
Regards,
Laurent Pinchart
Re: [PATCH v3 3/3] ARM: dts: r7s72100: Add watchdog timer
Hello!
On 3/2/2017 1:54 AM, Chris Brandt wrote:
Add watchdog timer support for RZ/A1.
For the RZ/A1, the only way to do a reset is to overflow the WDT, so this
is useful even if you don't need the watchdog functionality.
Signed-off-by: Chris Brandt
Reviewed-by: Geert Uytterhoeven
---
v3:
* added Reviewed-by
v2:
* changed "renesas,r7s72100-reset" to "renesas,r7s72100-wdt"
* changed "renesas,wdt-reset" to "renesas,rza-wdt"
* added interupt property (even though it is not used)
* added clocks property
---
arch/arm/boot/dts/r7s72100.dtsi | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index ed62e19..6ecee72 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -382,6 +382,13 @@
cache-level = <2>;
};
+ wdt: timer@fcfe {
Why not watchdog@...? That name is standardized in ePAPR...
+ compatible = "renesas,r7s72100-wdt", "renesas,rza-wdt";
+ reg = <0xfcfe 0x6>;
+ interrupts = ;
+ clocks = <&p0_clk>;
+ };
+
[...]
MBR, Sergei
Re: [PATCH] v4l: vsp1: Disable HSV formats on Gen3 hardware
Hi Laurent,
LGTM! :-)
On 28/02/17 23:08, Laurent Pinchart wrote:
> While all VSP instances can process HSV internally, on Gen3 hardware
> reading or writing HSV24 or HSV32 from/to memory causes the device to
> hang. Disable those pixel formats on Gen3 hardware.
>
> Signed-off-by: Laurent Pinchart
Reviewed-by: Kieran Bingham
> ---
> drivers/media/platform/vsp1/vsp1_pipe.c | 12 +---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c
> b/drivers/media/platform/vsp1/vsp1_pipe.c
> index 3f1acf68dc6e..35364f594e19 100644
> --- a/drivers/media/platform/vsp1/vsp1_pipe.c
> +++ b/drivers/media/platform/vsp1/vsp1_pipe.c
> @@ -157,9 +157,15 @@ const struct vsp1_format_info
> *vsp1_get_format_info(struct vsp1_device *vsp1,
> {
> unsigned int i;
>
> - /* Special case, the VYUY format is supported on Gen2 only. */
> - if (vsp1->info->gen != 2 && fourcc == V4L2_PIX_FMT_VYUY)
> - return NULL;
> + /* Special case, the VYUY and HSV formats are supported on Gen2 only. */
> + if (vsp1->info->gen != 2) {
> + switch (fourcc) {
> + case V4L2_PIX_FMT_VYUY:
> + case V4L2_PIX_FMT_HSV24:
> + case V4L2_PIX_FMT_HSV32:
> + return NULL;
> + }
> + }
>
> for (i = 0; i < ARRAY_SIZE(vsp1_video_formats); ++i) {
> const struct vsp1_format_info *info = &vsp1_video_formats[i];
>
