Re: [linux-sunxi] [PATCH v3 06/16] drm/sun4i: Release exclusive clock lock when disabling TCON

2018-03-08 Thread 'Ondřej Jirman' via linux-sunxi
Hi,

On Fri, Mar 09, 2018 at 07:19:33AM +0100, Jernej Škrabec wrote:
> Hi,
> 
> Dne petek, 09. marec 2018 ob 01:44:55 CET je Ondřej Jirman napisal(a):
> > Hi,
> > 
> > I've debugged this further and it seems that the code has incorrect
> > assumptions. See docs for mode_set_nofb.
> > 
> > https://elixir.bootlin.com/linux/v4.16-rc4/source/include/drm/drm_modeset_he
> > lper_vtables.h#L209
> >
> 
> Does this happen with https://github.com/jernejsk/linux-1/tree/h3_hdmi_v3 ?

Haven't tested, but it probably does, looking at the code. Try
restarting the X server a bunch of times.

> I also tested running X, but I don't remember having the issue.
>  
> > I've added traces to a few functions that call clk_.*exclusive.*()
> > functions, and I see this after a few restarts of Xorg server:
> > 
> > [0.783842] *** sun4i_tcon1_mode_set()
> > [0.783886] *** sun4i_tcon_channel_set_status(..., 1, true)
> > [6.881690] *** sun4i_tcon_channel_set_status(..., 1, false)
> > [6.881758] *** sun4i_tcon_channel_set_status(..., 1, true)
> > [   52.335085] *** sun4i_tcon_channel_set_status(..., 1, false)
> > [   52.335343] *** sun4i_tcon_channel_set_status(..., 1, true)
> > [   68.921079] *** sun4i_tcon_channel_set_status(..., 1, false)
> > [   68.921337] *** sun4i_tcon_channel_set_status(..., 1, true)
> > 
> > mode_set_nofb is called just once, yet sun4i_tcon_channel_set_status(..., 1,
> > false) is called multiple times, which leads to unbalanced calls to
> > clk_set_rate_exclusive and clk_rate_exclusive_put.
> > 
> > I don't know how to fix this.
> 
> Since there is no function to check if clock is protected, flag can be 
> introduced within TCON which is set when clock rate is protected and unset 
> when it is unprotected. That way we could track if clk_rate_exclusive_put() 
> needs to be called or not.

I think that will not help. Protection is set in sun4i_tcon1_mode_set()
and that's called only once, but sun4i_tcon_channel_set_status(..., 1, false)
can be called multiple times, so the first call would disable protection
and the clock would be unprotected from then on, even though the display
would be active.

Perhaps the protection needs to be enabled in sun4i_tcon_channel_set_status(...,
true).

regards,
  o.

> Best regards,
> Jernej
> 
> > 
> > regards,
> >   o.
> > 
> > On Fri, Mar 09, 2018 at 01:13:14AM +0100, 'Ondřej Jirman' via linux-sunxi 
> wrote:
> > > Hi Jernej,
> > > 
> > > On Thu, Mar 08, 2018 at 11:57:40PM +0100, Jernej Škrabec wrote:
> > > > Hi,
> > > > 
> > > > Dne četrtek, 08. marec 2018 ob 23:47:17 CET je Ondřej Jirman napisal(a):
> > > > > Hi,
> > > > > 
> > > > > On Thu, Mar 01, 2018 at 10:34:32PM +0100, Jernej Skrabec wrote:
> > > > > > Currently exclusive TCON clock lock is never released, which, for
> > > > > > example, prevents changing resolution on HDMI.
> > > > > > 
> > > > > > In order to fix that, release clock when disabling TCON. TCON is
> > > > > > always
> > > > > > disabled first before new mode is set.
> > > > > > 
> > > > > > Signed-off-by: Jernej Skrabec 
> > > > > > ---
> > > > > > 
> > > > > >  drivers/gpu/drm/sun4i/sun4i_tcon.c | 6 --
> > > > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index
> > > > > > 1d714c06ec9d..7f6c4125c89f
> > > > > > 100644
> > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > @@ -102,10 +102,12 @@ static void
> > > > > > sun4i_tcon_channel_set_status(struct
> > > > > > sun4i_tcon *tcon, int channel,>
> > > > > > 
> > > > > > return;
> > > > > > 
> > > > > > }
> > > > > > 
> > > > > > -   if (enabled)
> > > > > > +   if (enabled) {
> > > > > > 
> > > > > > clk_prepare_enable(clk);
> > > > > > 
> > > > > > -   else
> > > > > > +   } else {
> > > > > > +   clk_rate_exclusive_put(clk);
> > > > > > 
> > > > > > clk_disable_unprepare(clk);
> > > > > > 
> > > > > > +   }
> > > > > > 
> > > > > >  }
> > > > > 
> > > > > At least in the linux-next/master I can't see clk_rate_exclusive_get 
> call:
> > > > It is in drm-misc/drm-misc-fixes:
> > > > https://cgit.freedesktop.org/drm/drm-misc/commit/?h=drm-misc-fixes=79
> > > > d103a565d16b1893d990b2ee5e0fe71767759f
> > > > 
> > > > My patch is also applied in same branch, so there should be no issues
> > > > while
> > > > merging all those branches.
> > > > 
> > > > linux-next doesn't have either patches currently:
> > > > https://git.kernel.org/pub/
> > > > scm/linux/kernel/git/next/linux-next.git/log/drivers/gpu/drm/sun4i
> > > > 
> > > > This is issue only if you manually apply one patch without the other.
> > > 
> > > I have (and had) both patches applied. There's perhaps some code path,
> > > where sun4i_tcon_channel_set_status(..., false) is called in unbalanced
> > > way with regards to clk_set_rate_exclusive().
> > > 
> > > The 

Re: [linux-sunxi] [PATCH v3 06/16] drm/sun4i: Release exclusive clock lock when disabling TCON

2018-03-08 Thread Jernej Škrabec
Hi,

Dne petek, 09. marec 2018 ob 01:44:55 CET je Ondřej Jirman napisal(a):
> Hi,
> 
> I've debugged this further and it seems that the code has incorrect
> assumptions. See docs for mode_set_nofb.
> 
> https://elixir.bootlin.com/linux/v4.16-rc4/source/include/drm/drm_modeset_he
> lper_vtables.h#L209
>

Does this happen with https://github.com/jernejsk/linux-1/tree/h3_hdmi_v3 ?

I also tested running X, but I don't remember having the issue.
 
> I've added traces to a few functions that call clk_.*exclusive.*()
> functions, and I see this after a few restarts of Xorg server:
> 
> [0.783842] *** sun4i_tcon1_mode_set()
> [0.783886] *** sun4i_tcon_channel_set_status(..., 1, true)
> [6.881690] *** sun4i_tcon_channel_set_status(..., 1, false)
> [6.881758] *** sun4i_tcon_channel_set_status(..., 1, true)
> [   52.335085] *** sun4i_tcon_channel_set_status(..., 1, false)
> [   52.335343] *** sun4i_tcon_channel_set_status(..., 1, true)
> [   68.921079] *** sun4i_tcon_channel_set_status(..., 1, false)
> [   68.921337] *** sun4i_tcon_channel_set_status(..., 1, true)
> 
> mode_set_nofb is called just once, yet sun4i_tcon_channel_set_status(..., 1,
> false) is called multiple times, which leads to unbalanced calls to
> clk_set_rate_exclusive and clk_rate_exclusive_put.
> 
> I don't know how to fix this.

Since there is no function to check if clock is protected, flag can be 
introduced within TCON which is set when clock rate is protected and unset 
when it is unprotected. That way we could track if clk_rate_exclusive_put() 
needs to be called or not.

Best regards,
Jernej

> 
> regards,
>   o.
> 
> On Fri, Mar 09, 2018 at 01:13:14AM +0100, 'Ondřej Jirman' via linux-sunxi 
wrote:
> > Hi Jernej,
> > 
> > On Thu, Mar 08, 2018 at 11:57:40PM +0100, Jernej Škrabec wrote:
> > > Hi,
> > > 
> > > Dne četrtek, 08. marec 2018 ob 23:47:17 CET je Ondřej Jirman napisal(a):
> > > > Hi,
> > > > 
> > > > On Thu, Mar 01, 2018 at 10:34:32PM +0100, Jernej Skrabec wrote:
> > > > > Currently exclusive TCON clock lock is never released, which, for
> > > > > example, prevents changing resolution on HDMI.
> > > > > 
> > > > > In order to fix that, release clock when disabling TCON. TCON is
> > > > > always
> > > > > disabled first before new mode is set.
> > > > > 
> > > > > Signed-off-by: Jernej Skrabec 
> > > > > ---
> > > > > 
> > > > >  drivers/gpu/drm/sun4i/sun4i_tcon.c | 6 --
> > > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index
> > > > > 1d714c06ec9d..7f6c4125c89f
> > > > > 100644
> > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > @@ -102,10 +102,12 @@ static void
> > > > > sun4i_tcon_channel_set_status(struct
> > > > > sun4i_tcon *tcon, int channel,>
> > > > > 
> > > > >   return;
> > > > >   
> > > > >   }
> > > > > 
> > > > > - if (enabled)
> > > > > + if (enabled) {
> > > > > 
> > > > >   clk_prepare_enable(clk);
> > > > > 
> > > > > - else
> > > > > + } else {
> > > > > + clk_rate_exclusive_put(clk);
> > > > > 
> > > > >   clk_disable_unprepare(clk);
> > > > > 
> > > > > + }
> > > > > 
> > > > >  }
> > > > 
> > > > At least in the linux-next/master I can't see clk_rate_exclusive_get 
call:
> > > It is in drm-misc/drm-misc-fixes:
> > > https://cgit.freedesktop.org/drm/drm-misc/commit/?h=drm-misc-fixes=79
> > > d103a565d16b1893d990b2ee5e0fe71767759f
> > > 
> > > My patch is also applied in same branch, so there should be no issues
> > > while
> > > merging all those branches.
> > > 
> > > linux-next doesn't have either patches currently:
> > > https://git.kernel.org/pub/
> > > scm/linux/kernel/git/next/linux-next.git/log/drivers/gpu/drm/sun4i
> > > 
> > > This is issue only if you manually apply one patch without the other.
> > 
> > I have (and had) both patches applied. There's perhaps some code path,
> > where sun4i_tcon_channel_set_status(..., false) is called in unbalanced
> > way with regards to clk_set_rate_exclusive().
> > 
> > The issue occurs when starting Xorg. But Xorg works fine.
> > 
> > regards,
> > 
> >   o.
> >   
> > > Best regards,
> > > Jernej
> > > 
> > > > $ git grep 'exclusive' linux-next/master -- drivers/gpu/drm/sun4i
> > > > 
> > > > linux-next/master:sun4i_hdmi.h:* On sun5i the threshold is
> > > > exclusive,
> > > > i.e. does not include, linux-next/master:sun4i_tcon.c:
> > > > clk_rate_exclusive_put(clk); linux-next/master:sun4i_tcon.c:
> > > > clk_set_rate_exclusive(tcon->dclk, mode->crtc_clock * 1000);
> > > > linux-next/master:sun4i_tcon.c:   clk_set_rate_exclusive(tcon->sclk1,
> > > > mode->crtc_clock * 1000);
> > > > 
> > > > and the kernel complains too:
> > > > 
> > > > [  841.915161] [ cut here ]
> > > > [  841.915187] WARNING: CPU: 0 PID: 273 at
> > > > 

[linux-sunxi] Re: [PATCH v9 2/2] media: V3s: Add support for Allwinner CSI.

2018-03-08 Thread Yong
Hi,

On Tue, 6 Mar 2018 17:14:18 +0200
Sakari Ailus  wrote:

> Hi Yong,
> 
> Thanks for the patchset; please see my comments below.
> 
> On Tue, Mar 06, 2018 at 10:16:02AM +0800, Yong Deng wrote:
> > Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
> > interface and CSI1 is used for parallel interface. This is not
> > documented in datasheet but by test and guess.

...

> > +
> > +static const u32 supported_pixformats[] = {
> > +   V4L2_PIX_FMT_SBGGR8,
> > +   V4L2_PIX_FMT_SGBRG8,
> > +   V4L2_PIX_FMT_SGRBG8,
> > +   V4L2_PIX_FMT_SRGGB8,
> > +   V4L2_PIX_FMT_SBGGR10,
> > +   V4L2_PIX_FMT_SGBRG10,
> > +   V4L2_PIX_FMT_SGRBG10,
> > +   V4L2_PIX_FMT_SRGGB10,
> > +   V4L2_PIX_FMT_SBGGR12,
> > +   V4L2_PIX_FMT_SGBRG12,
> > +   V4L2_PIX_FMT_SGRBG12,
> > +   V4L2_PIX_FMT_SRGGB12,
> > +   V4L2_PIX_FMT_YUYV,
> > +   V4L2_PIX_FMT_YVYU,
> > +   V4L2_PIX_FMT_UYVY,
> > +   V4L2_PIX_FMT_VYUY,
> > +   V4L2_PIX_FMT_HM12,
> > +   V4L2_PIX_FMT_NV12,
> > +   V4L2_PIX_FMT_NV21,
> > +   V4L2_PIX_FMT_YUV420,
> > +   V4L2_PIX_FMT_YVU420,
> > +   V4L2_PIX_FMT_NV16,
> > +   V4L2_PIX_FMT_NV61,
> > +   V4L2_PIX_FMT_YUV422P,
> > +};
> 
> How about moving this where it's actually used? You'd also get rid of the
> function to obtain this list.

I think which formats are supported is determined by hardware (CSI).
And different SoCs may support different formats. The distinction will
be made in sun6i-csi.c.

> 
> > +
> > +static inline struct sun6i_csi_dev *sun6i_csi_to_dev(struct sun6i_csi *csi)
> > +{
> > +   return container_of(csi, struct sun6i_csi_dev, csi);
> > +}
> > +
> > +int sun6i_csi_get_supported_pixformats(struct sun6i_csi *csi,
> > +  const u32 **pixformats)
> > +{
> > +   if (pixformats != NULL)
> > +   *pixformats = supported_pixformats;
> > +
> > +   return ARRAY_SIZE(supported_pixformats);
> > +}
> > +
> > +/* TODO add 10&12 bit YUV, RGB support */
> > +bool sun6i_csi_is_format_support(struct sun6i_csi *csi,
> 
> s/support/supported/

OK.

> 
> > +u32 pixformat, u32 mbus_code)
> > +{
> > +   struct sun6i_csi_dev *sdev = sun6i_csi_to_dev(csi);
> > +
> > +   /*
> > +* Some video receivers have the ability to be compatible with
> > +* 8bit and 16bit bus width.
> > +* Identify the media bus format from device tree.
> > +*/
> > +   if ((sdev->csi.v4l2_ep.bus_type == V4L2_MBUS_PARALLEL
> > + || sdev->csi.v4l2_ep.bus_type == V4L2_MBUS_BT656)
> > +&& sdev->csi.v4l2_ep.bus.parallel.bus_width == 16) {
> > +   switch (pixformat) {
> > +   case V4L2_PIX_FMT_HM12:
> > +   case V4L2_PIX_FMT_NV12:
> > +   case V4L2_PIX_FMT_NV21:
> > +   case V4L2_PIX_FMT_NV16:
> > +   case V4L2_PIX_FMT_NV61:
> > +   case V4L2_PIX_FMT_YUV420:
> > +   case V4L2_PIX_FMT_YVU420:
> > +   case V4L2_PIX_FMT_YUV422P:
> > +   switch (mbus_code) {
> > +   case MEDIA_BUS_FMT_UYVY8_1X16:
> > +   case MEDIA_BUS_FMT_VYUY8_1X16:
> > +   case MEDIA_BUS_FMT_YUYV8_1X16:
> > +   case MEDIA_BUS_FMT_YVYU8_1X16:
> > +   return true;
> > +   default:
> > +   dev_dbg(sdev->dev, "Unsupported mbus code: 
> > 0x%x\n",
> > +   mbus_code);
> > +   break;
> > +   }
> > +   break;
> > +   default:
> > +   dev_dbg(sdev->dev, "Unsupported pixformat: 0x%x\n",
> > +   pixformat);
> > +   break;
> > +   }
> > +   return false;
> > +   }
> > +
> > +   switch (pixformat) {
> > +   case V4L2_PIX_FMT_SBGGR8:
> > +   return (mbus_code == MEDIA_BUS_FMT_SBGGR8_1X8);
> > +   case V4L2_PIX_FMT_SGBRG8:
> > +   return (mbus_code == MEDIA_BUS_FMT_SGBRG8_1X8);
> > +   case V4L2_PIX_FMT_SGRBG8:
> > +   return (mbus_code == MEDIA_BUS_FMT_SGRBG8_1X8);
> > +   case V4L2_PIX_FMT_SRGGB8:
> > +   return (mbus_code == MEDIA_BUS_FMT_SRGGB8_1X8);
> > +   case V4L2_PIX_FMT_SBGGR10:
> > +   return (mbus_code == MEDIA_BUS_FMT_SBGGR10_1X10);
> > +   case V4L2_PIX_FMT_SGBRG10:
> > +   return (mbus_code == MEDIA_BUS_FMT_SGBRG10_1X10);
> > +   case V4L2_PIX_FMT_SGRBG10:
> > +   return (mbus_code == MEDIA_BUS_FMT_SGRBG10_1X10);
> > +   case V4L2_PIX_FMT_SRGGB10:
> > +   return (mbus_code == MEDIA_BUS_FMT_SRGGB10_1X10);
> > +   case V4L2_PIX_FMT_SBGGR12:
> > +   return (mbus_code == MEDIA_BUS_FMT_SBGGR12_1X12);
> > +   case V4L2_PIX_FMT_SGBRG12:
> > +   return (mbus_code == MEDIA_BUS_FMT_SGBRG12_1X12);
> > +   case V4L2_PIX_FMT_SGRBG12:
> > +   return (mbus_code == MEDIA_BUS_FMT_SGRBG12_1X12);
> > +   case V4L2_PIX_FMT_SRGGB12:
> > +   return (mbus_code == MEDIA_BUS_FMT_SRGGB12_1X12);
> > +
> > +  

Re: [linux-sunxi] [PATCH v3 06/16] drm/sun4i: Release exclusive clock lock when disabling TCON

2018-03-08 Thread 'Ondřej Jirman' via linux-sunxi
Hi,

I've debugged this further and it seems that the code has incorrect assumptions.
See docs for mode_set_nofb.

https://elixir.bootlin.com/linux/v4.16-rc4/source/include/drm/drm_modeset_helper_vtables.h#L209

I've added traces to a few functions that call clk_.*exclusive.*() functions,
and I see this after a few restarts of Xorg server:

[0.783842] *** sun4i_tcon1_mode_set()
[0.783886] *** sun4i_tcon_channel_set_status(..., 1, true)
[6.881690] *** sun4i_tcon_channel_set_status(..., 1, false)
[6.881758] *** sun4i_tcon_channel_set_status(..., 1, true)
[   52.335085] *** sun4i_tcon_channel_set_status(..., 1, false)
[   52.335343] *** sun4i_tcon_channel_set_status(..., 1, true)
[   68.921079] *** sun4i_tcon_channel_set_status(..., 1, false)
[   68.921337] *** sun4i_tcon_channel_set_status(..., 1, true)

mode_set_nofb is called just once, yet sun4i_tcon_channel_set_status(..., 1, 
false)
is called multiple times, which leads to unbalanced calls to 
clk_set_rate_exclusive
and clk_rate_exclusive_put.

I don't know how to fix this.

regards,
  o.

On Fri, Mar 09, 2018 at 01:13:14AM +0100, 'Ondřej Jirman' via linux-sunxi wrote:
> Hi Jernej,
> 
> On Thu, Mar 08, 2018 at 11:57:40PM +0100, Jernej Škrabec wrote:
> > Hi,
> > 
> > Dne četrtek, 08. marec 2018 ob 23:47:17 CET je Ondřej Jirman napisal(a):
> > > Hi,
> > > 
> > > On Thu, Mar 01, 2018 at 10:34:32PM +0100, Jernej Skrabec wrote:
> > > > Currently exclusive TCON clock lock is never released, which, for
> > > > example, prevents changing resolution on HDMI.
> > > > 
> > > > In order to fix that, release clock when disabling TCON. TCON is always
> > > > disabled first before new mode is set.
> > > > 
> > > > Signed-off-by: Jernej Skrabec 
> > > > ---
> > > > 
> > > >  drivers/gpu/drm/sun4i/sun4i_tcon.c | 6 --
> > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 1d714c06ec9d..7f6c4125c89f
> > > > 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > @@ -102,10 +102,12 @@ static void sun4i_tcon_channel_set_status(struct
> > > > sun4i_tcon *tcon, int channel,> 
> > > > return;
> > > > 
> > > > }
> > > > 
> > > > -   if (enabled)
> > > > +   if (enabled) {
> > > > 
> > > > clk_prepare_enable(clk);
> > > > 
> > > > -   else
> > > > +   } else {
> > > > +   clk_rate_exclusive_put(clk);
> > > > 
> > > > clk_disable_unprepare(clk);
> > > > 
> > > > +   }
> > > > 
> > > >  }
> > > 
> > > At least in the linux-next/master I can't see clk_rate_exclusive_get call:
> > > 
> > 
> > It is in drm-misc/drm-misc-fixes:
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?h=drm-misc-fixes=79d103a565d16b1893d990b2ee5e0fe71767759f
> > 
> > My patch is also applied in same branch, so there should be no issues while 
> > merging all those branches.
> > 
> > linux-next doesn't have either patches currently: 
> > https://git.kernel.org/pub/
> > scm/linux/kernel/git/next/linux-next.git/log/drivers/gpu/drm/sun4i
> > 
> > This is issue only if you manually apply one patch without the other.
> 
> I have (and had) both patches applied. There's perhaps some code path,
> where sun4i_tcon_channel_set_status(..., false) is called in unbalanced
> way with regards to clk_set_rate_exclusive().
> 
> The issue occurs when starting Xorg. But Xorg works fine.
> 
> regards,
>   o.
> 
> > Best regards,
> > Jernej
> > 
> > > $ git grep 'exclusive' linux-next/master -- drivers/gpu/drm/sun4i
> > > 
> > > linux-next/master:sun4i_hdmi.h:* On sun5i the threshold is exclusive,
> > > i.e. does not include, linux-next/master:sun4i_tcon.c:  
> > > clk_rate_exclusive_put(clk); linux-next/master:sun4i_tcon.c:  
> > > clk_set_rate_exclusive(tcon->dclk, mode->crtc_clock * 1000);
> > > linux-next/master:sun4i_tcon.c:   clk_set_rate_exclusive(tcon->sclk1,
> > > mode->crtc_clock * 1000);
> > > 
> > > and the kernel complains too:
> > > 
> > > [  841.915161] [ cut here ]
> > > [  841.915187] WARNING: CPU: 0 PID: 273 at
> > > /workspace/megous.com/orangepi-pc/linux-4.16/drivers/clk/clk.c:608
> > > clk_rate_exclusive_put+0x48/0x4c [  841.915194] CPU: 0 PID: 273 Comm: Xorg
> > > Not tainted 4.16.0-rc4-00268-gbac2ecf73ed0 #13 [  841.915196] Hardware
> > > name: Allwinner sun8i Family
> > > [  841.915217] [] (unwind_backtrace) from []
> > > (show_stack+0x10/0x14) [  841.915228] [] (show_stack) from
> > > [] (dump_stack+0x88/0x9c) [  841.915237] []
> > > (dump_stack) from [] (__warn+0xd4/0xf0) [  841.915244]
> > > [] (__warn) from [] (warn_slowpath_null+0x40/0x48) [ 
> > > 841.915250] [] (warn_slowpath_null) from []
> > > (clk_rate_exclusive_put+0x48/0x4c) [  841.915261] []
> > > (clk_rate_exclusive_put) from []
> > > (sun4i_tcon_set_status+0x178/0x2f0) [  841.915269] []
> > > 

Re: [linux-sunxi] [PATCH v3 06/16] drm/sun4i: Release exclusive clock lock when disabling TCON

2018-03-08 Thread 'Ondřej Jirman' via linux-sunxi
Hi Jernej,

On Thu, Mar 08, 2018 at 11:57:40PM +0100, Jernej Škrabec wrote:
> Hi,
> 
> Dne četrtek, 08. marec 2018 ob 23:47:17 CET je Ondřej Jirman napisal(a):
> > Hi,
> > 
> > On Thu, Mar 01, 2018 at 10:34:32PM +0100, Jernej Skrabec wrote:
> > > Currently exclusive TCON clock lock is never released, which, for
> > > example, prevents changing resolution on HDMI.
> > > 
> > > In order to fix that, release clock when disabling TCON. TCON is always
> > > disabled first before new mode is set.
> > > 
> > > Signed-off-by: Jernej Skrabec 
> > > ---
> > > 
> > >  drivers/gpu/drm/sun4i/sun4i_tcon.c | 6 --
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 1d714c06ec9d..7f6c4125c89f
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > @@ -102,10 +102,12 @@ static void sun4i_tcon_channel_set_status(struct
> > > sun4i_tcon *tcon, int channel,> 
> > >   return;
> > >   
> > >   }
> > > 
> > > - if (enabled)
> > > + if (enabled) {
> > > 
> > >   clk_prepare_enable(clk);
> > > 
> > > - else
> > > + } else {
> > > + clk_rate_exclusive_put(clk);
> > > 
> > >   clk_disable_unprepare(clk);
> > > 
> > > + }
> > > 
> > >  }
> > 
> > At least in the linux-next/master I can't see clk_rate_exclusive_get call:
> > 
> 
> It is in drm-misc/drm-misc-fixes:
> https://cgit.freedesktop.org/drm/drm-misc/commit/?h=drm-misc-fixes=79d103a565d16b1893d990b2ee5e0fe71767759f
> 
> My patch is also applied in same branch, so there should be no issues while 
> merging all those branches.
> 
> linux-next doesn't have either patches currently: https://git.kernel.org/pub/
> scm/linux/kernel/git/next/linux-next.git/log/drivers/gpu/drm/sun4i
> 
> This is issue only if you manually apply one patch without the other.

I have (and had) both patches applied. There's perhaps some code path,
where sun4i_tcon_channel_set_status(..., false) is called in unbalanced
way with regards to clk_set_rate_exclusive().

The issue occurs when starting Xorg. But Xorg works fine.

regards,
  o.

> Best regards,
> Jernej
> 
> > $ git grep 'exclusive' linux-next/master -- drivers/gpu/drm/sun4i
> > 
> > linux-next/master:sun4i_hdmi.h:* On sun5i the threshold is exclusive,
> > i.e. does not include, linux-next/master:sun4i_tcon.c:  
> > clk_rate_exclusive_put(clk); linux-next/master:sun4i_tcon.c:  
> > clk_set_rate_exclusive(tcon->dclk, mode->crtc_clock * 1000);
> > linux-next/master:sun4i_tcon.c:   clk_set_rate_exclusive(tcon->sclk1,
> > mode->crtc_clock * 1000);
> > 
> > and the kernel complains too:
> > 
> > [  841.915161] [ cut here ]
> > [  841.915187] WARNING: CPU: 0 PID: 273 at
> > /workspace/megous.com/orangepi-pc/linux-4.16/drivers/clk/clk.c:608
> > clk_rate_exclusive_put+0x48/0x4c [  841.915194] CPU: 0 PID: 273 Comm: Xorg
> > Not tainted 4.16.0-rc4-00268-gbac2ecf73ed0 #13 [  841.915196] Hardware
> > name: Allwinner sun8i Family
> > [  841.915217] [] (unwind_backtrace) from []
> > (show_stack+0x10/0x14) [  841.915228] [] (show_stack) from
> > [] (dump_stack+0x88/0x9c) [  841.915237] []
> > (dump_stack) from [] (__warn+0xd4/0xf0) [  841.915244]
> > [] (__warn) from [] (warn_slowpath_null+0x40/0x48) [ 
> > 841.915250] [] (warn_slowpath_null) from []
> > (clk_rate_exclusive_put+0x48/0x4c) [  841.915261] []
> > (clk_rate_exclusive_put) from []
> > (sun4i_tcon_set_status+0x178/0x2f0) [  841.915269] []
> > (sun4i_tcon_set_status) from []
> > (sun4i_crtc_atomic_disable+0x7c/0xe4) [  841.915279] []
> > (sun4i_crtc_atomic_disable) from []
> > (drm_atomic_helper_commit_modeset_disables+0x390/0x46c) [  841.915288]
> > [] (drm_atomic_helper_commit_modeset_disables) from []
> > (drm_atomic_helper_commit_tail_rpm+0x18/0x64) [  841.915295] []
> > (drm_atomic_helper_commit_tail_rpm) from []
> > (commit_tail+0x40/0x6c) [  841.915302] [] (commit_tail) from
> > [] (drm_atomic_helper_commit+0xbc/0x128) [  841.915311]
> > [] (drm_atomic_helper_commit) from []
> > (restore_fbdev_mode_atomic+0x100/0x1fc) [  841.915319] []
> > (restore_fbdev_mode_atomic) from []
> > (drm_fb_helper_dpms+0x50/0x130) [  841.915328] []
> > (drm_fb_helper_dpms) from [] (drm_fb_helper_blank+0x54/0x90) [ 
> > 841.915337] [] (drm_fb_helper_blank) from []
> > (fb_blank+0x54/0xa8) [  841.915343] [] (fb_blank) from
> > [] (do_fb_ioctl+0x360/0x62c) [  841.915351] []
> > (do_fb_ioctl) from [] (do_vfs_ioctl+0x9c/0x774) [  841.915358]
> > [] (do_vfs_ioctl) from [] (SyS_ioctl+0x34/0x58) [ 
> > 841.915364] [] (SyS_ioctl) from []
> > (ret_fast_syscall+0x0/0x4c) [  841.915368] Exception stack(0xe5fc5fa8 to
> > 0xe5fc5ff0)
> > [  841.915373] 5fa0:   00674f10 00675460 000b 4611
> > 0001  [  841.915379] 5fc0: 00674f10 00675460 0001 0036
> > 00650474  006504a4 00675df8 [  841.915382] 5fe0: b61a502c be8acb2c
> > 

Re: [linux-sunxi] [PATCH v3 06/16] drm/sun4i: Release exclusive clock lock when disabling TCON

2018-03-08 Thread Jernej Škrabec
Hi,

Dne četrtek, 08. marec 2018 ob 23:47:17 CET je Ondřej Jirman napisal(a):
> Hi,
> 
> On Thu, Mar 01, 2018 at 10:34:32PM +0100, Jernej Skrabec wrote:
> > Currently exclusive TCON clock lock is never released, which, for
> > example, prevents changing resolution on HDMI.
> > 
> > In order to fix that, release clock when disabling TCON. TCON is always
> > disabled first before new mode is set.
> > 
> > Signed-off-by: Jernej Skrabec 
> > ---
> > 
> >  drivers/gpu/drm/sun4i/sun4i_tcon.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 1d714c06ec9d..7f6c4125c89f
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > @@ -102,10 +102,12 @@ static void sun4i_tcon_channel_set_status(struct
> > sun4i_tcon *tcon, int channel,> 
> > return;
> > 
> > }
> > 
> > -   if (enabled)
> > +   if (enabled) {
> > 
> > clk_prepare_enable(clk);
> > 
> > -   else
> > +   } else {
> > +   clk_rate_exclusive_put(clk);
> > 
> > clk_disable_unprepare(clk);
> > 
> > +   }
> > 
> >  }
> 
> At least in the linux-next/master I can't see clk_rate_exclusive_get call:
> 

It is in drm-misc/drm-misc-fixes:
https://cgit.freedesktop.org/drm/drm-misc/commit/?h=drm-misc-fixes=79d103a565d16b1893d990b2ee5e0fe71767759f

My patch is also applied in same branch, so there should be no issues while 
merging all those branches.

linux-next doesn't have either patches currently: https://git.kernel.org/pub/
scm/linux/kernel/git/next/linux-next.git/log/drivers/gpu/drm/sun4i

This is issue only if you manually apply one patch without the other.

Best regards,
Jernej

> $ git grep 'exclusive' linux-next/master -- drivers/gpu/drm/sun4i
> 
> linux-next/master:sun4i_hdmi.h:* On sun5i the threshold is exclusive,
> i.e. does not include, linux-next/master:sun4i_tcon.c:  
> clk_rate_exclusive_put(clk); linux-next/master:sun4i_tcon.c:  
> clk_set_rate_exclusive(tcon->dclk, mode->crtc_clock * 1000);
> linux-next/master:sun4i_tcon.c:   clk_set_rate_exclusive(tcon->sclk1,
> mode->crtc_clock * 1000);
> 
> and the kernel complains too:
> 
> [  841.915161] [ cut here ]
> [  841.915187] WARNING: CPU: 0 PID: 273 at
> /workspace/megous.com/orangepi-pc/linux-4.16/drivers/clk/clk.c:608
> clk_rate_exclusive_put+0x48/0x4c [  841.915194] CPU: 0 PID: 273 Comm: Xorg
> Not tainted 4.16.0-rc4-00268-gbac2ecf73ed0 #13 [  841.915196] Hardware
> name: Allwinner sun8i Family
> [  841.915217] [] (unwind_backtrace) from []
> (show_stack+0x10/0x14) [  841.915228] [] (show_stack) from
> [] (dump_stack+0x88/0x9c) [  841.915237] []
> (dump_stack) from [] (__warn+0xd4/0xf0) [  841.915244]
> [] (__warn) from [] (warn_slowpath_null+0x40/0x48) [ 
> 841.915250] [] (warn_slowpath_null) from []
> (clk_rate_exclusive_put+0x48/0x4c) [  841.915261] []
> (clk_rate_exclusive_put) from []
> (sun4i_tcon_set_status+0x178/0x2f0) [  841.915269] []
> (sun4i_tcon_set_status) from []
> (sun4i_crtc_atomic_disable+0x7c/0xe4) [  841.915279] []
> (sun4i_crtc_atomic_disable) from []
> (drm_atomic_helper_commit_modeset_disables+0x390/0x46c) [  841.915288]
> [] (drm_atomic_helper_commit_modeset_disables) from []
> (drm_atomic_helper_commit_tail_rpm+0x18/0x64) [  841.915295] []
> (drm_atomic_helper_commit_tail_rpm) from []
> (commit_tail+0x40/0x6c) [  841.915302] [] (commit_tail) from
> [] (drm_atomic_helper_commit+0xbc/0x128) [  841.915311]
> [] (drm_atomic_helper_commit) from []
> (restore_fbdev_mode_atomic+0x100/0x1fc) [  841.915319] []
> (restore_fbdev_mode_atomic) from []
> (drm_fb_helper_dpms+0x50/0x130) [  841.915328] []
> (drm_fb_helper_dpms) from [] (drm_fb_helper_blank+0x54/0x90) [ 
> 841.915337] [] (drm_fb_helper_blank) from []
> (fb_blank+0x54/0xa8) [  841.915343] [] (fb_blank) from
> [] (do_fb_ioctl+0x360/0x62c) [  841.915351] []
> (do_fb_ioctl) from [] (do_vfs_ioctl+0x9c/0x774) [  841.915358]
> [] (do_vfs_ioctl) from [] (SyS_ioctl+0x34/0x58) [ 
> 841.915364] [] (SyS_ioctl) from []
> (ret_fast_syscall+0x0/0x4c) [  841.915368] Exception stack(0xe5fc5fa8 to
> 0xe5fc5ff0)
> [  841.915373] 5fa0:   00674f10 00675460 000b 4611
> 0001  [  841.915379] 5fc0: 00674f10 00675460 0001 0036
> 00650474  006504a4 00675df8 [  841.915382] 5fe0: b61a502c be8acb2c
> b6192c38 b6b152ec
> [  841.915386] ---[ end trace fa81b956197707f8 ]---
> 
> It looks like clk_rate_exclusive_put inside sun4i_tcon_channel_set_status is
> called without first calling some function that would call
> clk_set_rate_exclusive, leading to unbalanced get/put calls.
> 
> regards,
>   o.
> 
> >  static void sun4i_tcon_lvds_set_status(struct sun4i_tcon *tcon,
> > 
> > --
> > 2.16.2
> > 
> > --
> > You received this message because you are subscribed to the Google Groups
> > "linux-sunxi" group. To unsubscribe from this group 

Re: [linux-sunxi] [PATCH v3 06/16] drm/sun4i: Release exclusive clock lock when disabling TCON

2018-03-08 Thread 'Ondřej Jirman' via linux-sunxi
Hi,

On Thu, Mar 01, 2018 at 10:34:32PM +0100, Jernej Skrabec wrote:
> Currently exclusive TCON clock lock is never released, which, for
> example, prevents changing resolution on HDMI.
> 
> In order to fix that, release clock when disabling TCON. TCON is always
> disabled first before new mode is set.
> 
> Signed-off-by: Jernej Skrabec 
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 1d714c06ec9d..7f6c4125c89f 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -102,10 +102,12 @@ static void sun4i_tcon_channel_set_status(struct 
> sun4i_tcon *tcon, int channel,
>   return;
>   }
>  
> - if (enabled)
> + if (enabled) {
>   clk_prepare_enable(clk);
> - else
> + } else {
> + clk_rate_exclusive_put(clk);
>   clk_disable_unprepare(clk);
> + }
>  }

At least in the linux-next/master I can't see clk_rate_exclusive_get call:

$ git grep 'exclusive' linux-next/master -- drivers/gpu/drm/sun4i

linux-next/master:sun4i_hdmi.h:* On sun5i the threshold is exclusive, i.e. 
does not include,
linux-next/master:sun4i_tcon.c:   clk_rate_exclusive_put(clk);
linux-next/master:sun4i_tcon.c:   clk_set_rate_exclusive(tcon->dclk, 
mode->crtc_clock * 1000);
linux-next/master:sun4i_tcon.c:   clk_set_rate_exclusive(tcon->sclk1, 
mode->crtc_clock * 1000);

and the kernel complains too:

[  841.915161] [ cut here ]
[  841.915187] WARNING: CPU: 0 PID: 273 at 
/workspace/megous.com/orangepi-pc/linux-4.16/drivers/clk/clk.c:608 
clk_rate_exclusive_put+0x48/0x4c
[  841.915194] CPU: 0 PID: 273 Comm: Xorg Not tainted 
4.16.0-rc4-00268-gbac2ecf73ed0 #13
[  841.915196] Hardware name: Allwinner sun8i Family
[  841.915217] [] (unwind_backtrace) from [] 
(show_stack+0x10/0x14)
[  841.915228] [] (show_stack) from [] 
(dump_stack+0x88/0x9c)
[  841.915237] [] (dump_stack) from [] (__warn+0xd4/0xf0)
[  841.915244] [] (__warn) from [] 
(warn_slowpath_null+0x40/0x48)
[  841.915250] [] (warn_slowpath_null) from [] 
(clk_rate_exclusive_put+0x48/0x4c)
[  841.915261] [] (clk_rate_exclusive_put) from [] 
(sun4i_tcon_set_status+0x178/0x2f0)
[  841.915269] [] (sun4i_tcon_set_status) from [] 
(sun4i_crtc_atomic_disable+0x7c/0xe4)
[  841.915279] [] (sun4i_crtc_atomic_disable) from [] 
(drm_atomic_helper_commit_modeset_disables+0x390/0x46c)
[  841.915288] [] (drm_atomic_helper_commit_modeset_disables) from 
[] (drm_atomic_helper_commit_tail_rpm+0x18/0x64)
[  841.915295] [] (drm_atomic_helper_commit_tail_rpm) from 
[] (commit_tail+0x40/0x6c)
[  841.915302] [] (commit_tail) from [] 
(drm_atomic_helper_commit+0xbc/0x128)
[  841.915311] [] (drm_atomic_helper_commit) from [] 
(restore_fbdev_mode_atomic+0x100/0x1fc)
[  841.915319] [] (restore_fbdev_mode_atomic) from [] 
(drm_fb_helper_dpms+0x50/0x130)
[  841.915328] [] (drm_fb_helper_dpms) from [] 
(drm_fb_helper_blank+0x54/0x90)
[  841.915337] [] (drm_fb_helper_blank) from [] 
(fb_blank+0x54/0xa8)
[  841.915343] [] (fb_blank) from [] 
(do_fb_ioctl+0x360/0x62c)
[  841.915351] [] (do_fb_ioctl) from [] 
(do_vfs_ioctl+0x9c/0x774)
[  841.915358] [] (do_vfs_ioctl) from [] 
(SyS_ioctl+0x34/0x58)
[  841.915364] [] (SyS_ioctl) from [] 
(ret_fast_syscall+0x0/0x4c)
[  841.915368] Exception stack(0xe5fc5fa8 to 0xe5fc5ff0)
[  841.915373] 5fa0:   00674f10 00675460 000b 4611 
0001 
[  841.915379] 5fc0: 00674f10 00675460 0001 0036 00650474  
006504a4 00675df8
[  841.915382] 5fe0: b61a502c be8acb2c b6192c38 b6b152ec
[  841.915386] ---[ end trace fa81b956197707f8 ]---

It looks like clk_rate_exclusive_put inside sun4i_tcon_channel_set_status is
called without first calling some function that would call
clk_set_rate_exclusive, leading to unbalanced get/put calls.

regards,
  o.

>  static void sun4i_tcon_lvds_set_status(struct sun4i_tcon *tcon,
> -- 
> 2.16.2
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-sunxi+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [linux-sunxi] Re: DRM fb issue - Kernel 4.15.7

2018-03-08 Thread Jernej Škrabec
Hi,

Dne četrtek, 08. marec 2018 ob 15:15:39 CET je @lex napisal(a):
> These are my
> changes: https://gist.github.com/avafinger/40e7751f5d8e8fbe0225e3ad4c5da0bf
> 
> On Thursday, March 8, 2018 at 10:57:13 AM UTC-3, @lex wrote:
> > Ok, will test  Ondřej and your branch. This can take some time, I think
> > this weekend I can build and test both.

I think I noticed what the issue is so you don't need to test everything. 
Ondřej branch is actually using my mainline patches, but unfortunately first 
version. This has serious clock bug, which is fixed in later versions.

All you have to do is replace this commit:
https://github.com/megous/linux/commit/
fbae2e17a90818b0845c17c1d544978a3b05487d

with this one:
https://github.com/jernejsk/linux-1/commit/
cd6807cde15f7c87dd0bce18bf114dcef6796cf9

That should do it.

Best regards,
Jernej

> > 
> > But this raised some questions:
> > a) How is this going to help you guys fix the stable 4.15.y ?
> > b) I applied the significant changes on DRM and sun4i (that i think would
> > fix) on my 4.15 based on Ondřej 4.16 branch without much help, still same
> > problem.
> > clk.c (4.16) has received some changes and I have not touched it.
> > 
> > [   58.647727] [ cut here ]
> > 
> >> [   58.647802] WARNING: CPU: 1 PID: 72 at
> >> 
> >>> drivers/gpu/drm/sun4i/sun4i_crtc.c:57 sun4i_crtc_atomic_begin+0x6c/0x70
> >> 
> >> [   58.647814] Modules linked in: ir_lirc_codec lirc_dev sunxi_cir
> >> 
> >>> brcmfmac brcmutil g_serial ipv6
> >> 
> >> [   58.647909] CPU: 1 PID: 72 Comm: irq/46-1ee. Not tainted
> >> 
> >>> 4.15.7-h2-2 #10
> >> 
> >> [   58.647920] Hardware name: Allwinner sun8i Family
> >> 
> >> [   58.647999] [] (unwind_backtrace) from []
> >> 
> >>> (show_stack+0x10/0x14)
> >> 
> >> [   58.648050] [] (show_stack) from []
> >> 
> >>> (dump_stack+0x88/0x9c)
> >> 
> >> [   58.648092] [] (dump_stack) from []
> >> 
> >>> (__warn+0xdc/0xf4)
> >> 
> >> [   58.648128] [] (__warn) from []
> >> 
> >>> (warn_slowpath_null+0x40/0x48)
> >> 
> >> [   58.648164] [] (warn_slowpath_null) from []
> >> 
> >>> (sun4i_crtc_atomic_begin+0x6c/0x70)
> >> 
> >> [   58.648215] [] (sun4i_crtc_atomic_begin) from []
> >> 
> >>> (drm_atomic_helper_commit_planes+0x74/0x2ac)
> >> 
> >> [   58.648264] [] (drm_atomic_helper_commit_planes) from
> >> 
> >>> [] (drm_atomic_helper_commit_tail+0x28/0x64)
> >> 
> >> [   58.648311] [] (drm_atomic_helper_commit_tail) from
> >> 
> >>> [] (commit_tail+0x80/0x84)
> >> 
> >> [   58.648355] [] (commit_tail) from []
> >> 
> >>> (drm_atomic_helper_commit+0x120/0x124)
> >> 
> >> [   58.648399] [] (drm_atomic_helper_commit) from []
> >> 
> >>> (restore_fbdev_mode_atomic+0x178/0x1d4)
> >> 
> >> [   58.648442] [] (restore_fbdev_mode_atomic) from []
> >> 
> >>> (drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0)
> >> 
> >> [   58.648482] [] (drm_fb_helper_restore_fbdev_mode_unlocked)
> >> 
> >>> from [] (drm_fb_helper_set_par+0x30/0x60)
> >> 
> >> [   58.648521] [] (drm_fb_helper_set_par) from []
> >> 
> >>> (drm_fb_helper_hotplug_event.part.11+0xa8/0xb0)
> >> 
> >> [   58.648562] [] (drm_fb_helper_hotplug_event.part.11) from
> >> 
> >>> [] (drm_helper_hpd_irq_event+0xf0/0xfc)
> >> 
> >> [   58.648609] [] (drm_helper_hpd_irq_event) from []
> >> 
> >>> (dw_hdmi_irq+0xfc/0x184)
> >> 
> >> [   58.648655] [] (dw_hdmi_irq) from []
> >> 
> >>> (irq_thread_fn+0x1c/0x54)
> >> 
> >> [   58.648692] [] (irq_thread_fn) from []
> >> 
> >>> (irq_thread+0x138/0x1f8)
> >> 
> >> [   58.648732] [] (irq_thread) from []
> >> 
> >>> (kthread+0x148/0x150)
> >> 
> >> [   58.648777] [] (kthread) from []
> >> 
> >>> (ret_from_fork+0x14/0x2c)
> >> 
> >> [   58.648794] ---[ end trace db8b21de0cfe2bd6 ]---
> >> 
> >> [   58.904890] [ cut here ]
> >> 
> >> [   58.905010] WARNING: CPU: 1 PID: 72 at
> >> 
> >>> drivers/gpu/drm/drm_vblank.c:1015
> >>> drm_atomic_helper_wait_for_vblanks.part.1+0x10c/0x290
> >> 
> >> [   58.905030] Modules linked in: ir_lirc_codec lirc_dev sunxi_cir
> >> 
> >>> brcmfmac brcmutil g_serial ipv6
> >> 
> >> [   58.905207] CPU: 1 PID: 72 Comm: irq/46-1ee. Tainted: GW
> >> 
> >>>   4.15.7-h2-2 #10
> >> 
> >> [   58.905229] Hardware name: Allwinner sun8i Family
> >> 
> >> [   58.905346] [] (unwind_backtrace) from []
> >> 
> >>> (show_stack+0x10/0x14)
> >> 
> >> [   58.905439] [] (show_stack) from []
> >> 
> >>> (dump_stack+0x88/0x9c)
> >> 
> >> [   58.905515] [] (dump_stack) from []
> >> 
> >>> (__warn+0xdc/0xf4)
> >> 
> >> [   58.905583] [] (__warn) from []
> >> 
> >>> (warn_slowpath_null+0x40/0x48)
> >> 
> >> [   58.905668] [] (warn_slowpath_null) from []
> >> 
> >>> (drm_atomic_helper_wait_for_vblanks.part.1+0x10c/0x290)
> >> 
> >> [   58.905770] [] (drm_atomic_helper_wait_for_vblanks.part.1)
> >> 
> >>> from [] (drm_atomic_helper_commit_tail+0x54/0x64)
> >> 
> >> [   58.905860] [] (drm_atomic_helper_commit_tail) from
> >> 
> >>> [] (commit_tail+0x80/0x84)
> >> 
> >> [   58.905950] [] (commit_tail) from []
> 

Re: [linux-sunxi] Re: [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings

2018-03-08 Thread Andre Przywara
Hi,

On 08/03/18 14:37, Rob Herring wrote:
> On Thu, Mar 8, 2018 at 3:09 AM, Andre Przywara  wrote:
>> Hi,
>>
>> On 08/03/18 02:08, Rob Herring wrote:
>>> On Wed, Mar 07, 2018 at 02:07:18AM +, Andre Przywara wrote:
 The PWM controllers found in the Allwinner A64, H5 and H6 SoCs are fully
 compatible to the PWM controllers found in the A13 and H3.
>>>
>>> If fully compatible, then shouldn't there be a fallback to one of the
>>> existing compatible strings?
>>
>> Yes, that was the idea. I was already wondering how we would
>> differentiate that from "real" compatible strings, but couldn't find
>> convincing examples. So should that read:
>>
>> - compatible: should at least contain be one of:
>> - "allwinner,sun4i-a10-pwm"
>> - "allwinner,sun5i-a10s-pwm"
>> - "allwinner,sun5i-a13-pwm"
>> - "allwinner,sun7i-a20-pwm"
>> - "allwinner,sun8i-h3-pwm"
>>   May in addition contain one of:
>> - "allwinner,sun50i-a64-pwm"
>> - "allwinner,sun50i-h5-pwm"
>> - "allwinner,sun50i-h6-pwm"
> 
> I can't validate a dts is correct with it written like this. Just list
> each valid combination on each line:
> 
> "allwinner,sun8i-h3-pwm", "allwinner,sun50i-h6-pwm"
> 
> (Assuming "allwinner,sun50i-h6-pwm" was the first implementation and
> h3 has the same block).

Ah, OK, that's makes some sense. I didn't find too many examples outside
of root compatible nodes in the binding docs, but I will use that.

>> And this would possibly need to be amended once we need to actually use
>> one of those strings (to implement a quirk, for instance)?
> 
> The most specific compatible provides that. The driver matches on the
> least specific one until you have a quirk to implement.

Yeah, I was just wondering how a (non-Linux) driver author would induce
what strings are actually needed just by reading the binding. But
enumerating the combinations explicitly should solve this.

Thanks!
Andre.

>> Or is there some other way of reserving those compatible strings, which
>> are not expected to be matched in drivers directly?
> 
> The binding should just list all that are appropriate from the start
> and the driver is free to match on which ever string it wants.
> 
> Rob
> 

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[linux-sunxi] Re: [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings

2018-03-08 Thread Rob Herring
On Thu, Mar 8, 2018 at 3:09 AM, Andre Przywara  wrote:
> Hi,
>
> On 08/03/18 02:08, Rob Herring wrote:
>> On Wed, Mar 07, 2018 at 02:07:18AM +, Andre Przywara wrote:
>>> The PWM controllers found in the Allwinner A64, H5 and H6 SoCs are fully
>>> compatible to the PWM controllers found in the A13 and H3.
>>
>> If fully compatible, then shouldn't there be a fallback to one of the
>> existing compatible strings?
>
> Yes, that was the idea. I was already wondering how we would
> differentiate that from "real" compatible strings, but couldn't find
> convincing examples. So should that read:
>
> - compatible: should at least contain be one of:
> - "allwinner,sun4i-a10-pwm"
> - "allwinner,sun5i-a10s-pwm"
> - "allwinner,sun5i-a13-pwm"
> - "allwinner,sun7i-a20-pwm"
> - "allwinner,sun8i-h3-pwm"
>   May in addition contain one of:
> - "allwinner,sun50i-a64-pwm"
> - "allwinner,sun50i-h5-pwm"
> - "allwinner,sun50i-h6-pwm"

I can't validate a dts is correct with it written like this. Just list
each valid combination on each line:

"allwinner,sun8i-h3-pwm", "allwinner,sun50i-h6-pwm"

(Assuming "allwinner,sun50i-h6-pwm" was the first implementation and
h3 has the same block).

> And this would possibly need to be amended once we need to actually use
> one of those strings (to implement a quirk, for instance)?

The most specific compatible provides that. The driver matches on the
least specific one until you have a quirk to implement.

> Or is there some other way of reserving those compatible strings, which
> are not expected to be matched in drivers directly?

The binding should just list all that are appropriate from the start
and the driver is free to match on which ever string it wants.

Rob

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [linux-sunxi] Re: DRM fb issue - Kernel 4.15.7

2018-03-08 Thread @lex
These are my 
changes: https://gist.github.com/avafinger/40e7751f5d8e8fbe0225e3ad4c5da0bf



On Thursday, March 8, 2018 at 10:57:13 AM UTC-3, @lex wrote:
>
> Ok, will test  Ondřej and your branch. This can take some time, I think 
> this weekend I can build and test both.
>
> But this raised some questions:
> a) How is this going to help you guys fix the stable 4.15.y ?
> b) I applied the significant changes on DRM and sun4i (that i think would 
> fix) on my 4.15 based on Ondřej 4.16 branch without much help, still same 
> problem.
> clk.c (4.16) has received some changes and I have not touched it.
>
> [   58.647727] [ cut here ]
>>
>> [   58.647802] WARNING: CPU: 1 PID: 72 at 
>>> drivers/gpu/drm/sun4i/sun4i_crtc.c:57 sun4i_crtc_atomic_begin+0x6c/0x70
>>
>> [   58.647814] Modules linked in: ir_lirc_codec lirc_dev sunxi_cir 
>>> brcmfmac brcmutil g_serial ipv6
>>
>> [   58.647909] CPU: 1 PID: 72 Comm: irq/46-1ee. Not tainted 
>>> 4.15.7-h2-2 #10
>>
>> [   58.647920] Hardware name: Allwinner sun8i Family
>>
>> [   58.647999] [] (unwind_backtrace) from [] 
>>> (show_stack+0x10/0x14)
>>
>> [   58.648050] [] (show_stack) from [] 
>>> (dump_stack+0x88/0x9c)
>>
>> [   58.648092] [] (dump_stack) from [] 
>>> (__warn+0xdc/0xf4)
>>
>> [   58.648128] [] (__warn) from [] 
>>> (warn_slowpath_null+0x40/0x48)
>>
>> [   58.648164] [] (warn_slowpath_null) from [] 
>>> (sun4i_crtc_atomic_begin+0x6c/0x70)
>>
>> [   58.648215] [] (sun4i_crtc_atomic_begin) from [] 
>>> (drm_atomic_helper_commit_planes+0x74/0x2ac)
>>
>> [   58.648264] [] (drm_atomic_helper_commit_planes) from 
>>> [] (drm_atomic_helper_commit_tail+0x28/0x64)
>>
>> [   58.648311] [] (drm_atomic_helper_commit_tail) from 
>>> [] (commit_tail+0x80/0x84)
>>
>> [   58.648355] [] (commit_tail) from [] 
>>> (drm_atomic_helper_commit+0x120/0x124)
>>
>> [   58.648399] [] (drm_atomic_helper_commit) from [] 
>>> (restore_fbdev_mode_atomic+0x178/0x1d4)
>>
>> [   58.648442] [] (restore_fbdev_mode_atomic) from [] 
>>> (drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0)
>>
>> [   58.648482] [] (drm_fb_helper_restore_fbdev_mode_unlocked) 
>>> from [] (drm_fb_helper_set_par+0x30/0x60)
>>
>> [   58.648521] [] (drm_fb_helper_set_par) from [] 
>>> (drm_fb_helper_hotplug_event.part.11+0xa8/0xb0)
>>
>> [   58.648562] [] (drm_fb_helper_hotplug_event.part.11) from 
>>> [] (drm_helper_hpd_irq_event+0xf0/0xfc)
>>
>> [   58.648609] [] (drm_helper_hpd_irq_event) from [] 
>>> (dw_hdmi_irq+0xfc/0x184)
>>
>> [   58.648655] [] (dw_hdmi_irq) from [] 
>>> (irq_thread_fn+0x1c/0x54)
>>
>> [   58.648692] [] (irq_thread_fn) from [] 
>>> (irq_thread+0x138/0x1f8)
>>
>> [   58.648732] [] (irq_thread) from [] 
>>> (kthread+0x148/0x150)
>>
>> [   58.648777] [] (kthread) from [] 
>>> (ret_from_fork+0x14/0x2c)
>>
>> [   58.648794] ---[ end trace db8b21de0cfe2bd6 ]---
>>
>> [   58.904890] [ cut here ]
>>
>> [   58.905010] WARNING: CPU: 1 PID: 72 at 
>>> drivers/gpu/drm/drm_vblank.c:1015 
>>> drm_atomic_helper_wait_for_vblanks.part.1+0x10c/0x290
>>
>> [   58.905030] Modules linked in: ir_lirc_codec lirc_dev sunxi_cir 
>>> brcmfmac brcmutil g_serial ipv6
>>
>> [   58.905207] CPU: 1 PID: 72 Comm: irq/46-1ee. Tainted: GW  
>>>   4.15.7-h2-2 #10
>>
>> [   58.905229] Hardware name: Allwinner sun8i Family
>>
>> [   58.905346] [] (unwind_backtrace) from [] 
>>> (show_stack+0x10/0x14)
>>
>> [   58.905439] [] (show_stack) from [] 
>>> (dump_stack+0x88/0x9c)
>>
>> [   58.905515] [] (dump_stack) from [] 
>>> (__warn+0xdc/0xf4)
>>
>> [   58.905583] [] (__warn) from [] 
>>> (warn_slowpath_null+0x40/0x48)
>>
>> [   58.905668] [] (warn_slowpath_null) from [] 
>>> (drm_atomic_helper_wait_for_vblanks.part.1+0x10c/0x290)
>>
>> [   58.905770] [] (drm_atomic_helper_wait_for_vblanks.part.1) 
>>> from [] (drm_atomic_helper_commit_tail+0x54/0x64)
>>
>> [   58.905860] [] (drm_atomic_helper_commit_tail) from 
>>> [] (commit_tail+0x80/0x84)
>>
>> [   58.905950] [] (commit_tail) from [] 
>>> (drm_atomic_helper_commit+0x120/0x124)
>>
>> [   58.906034] [] (drm_atomic_helper_commit) from [] 
>>> (restore_fbdev_mode_atomic+0x178/0x1d4)
>>
>> [   58.906116] [] (restore_fbdev_mode_atomic) from [] 
>>> (drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0)
>>
>> [   58.906195] [] (drm_fb_helper_restore_fbdev_mode_unlocked) 
>>> from [] (drm_fb_helper_set_par+0x30/0x60)
>>
>> [   58.906273] [] (drm_fb_helper_set_par) from [] 
>>> (drm_fb_helper_hotplug_event.part.11+0xa8/0xb0)
>>
>> [   58.906353] [] (drm_fb_helper_hotplug_event.part.11) from 
>>> [] (drm_helper_hpd_irq_event+0xf0/0xfc)
>>
>> [   58.906445] [] (drm_helper_hpd_irq_event) from [] 
>>> (dw_hdmi_irq+0xfc/0x184)
>>
>> [   58.906530] [] (dw_hdmi_irq) from [] 
>>> (irq_thread_fn+0x1c/0x54)
>>
>> [   58.906601] [] (irq_thread_fn) from [] 
>>> (irq_thread+0x138/0x1f8)
>>
>> [   58.906674] [] (irq_thread) from [] 
>>> (kthread+0x148/0x150)
>>
>> [   58.906756] [] (kthread) from [] 
>>> (ret_from_fork+0x14/0x2c)
>>

[linux-sunxi] Re: Question about a quirky (DesignWare) PCIe RC in Allwinner H6

2018-03-08 Thread Icenowy Zheng
在 2018-03-08四的 21:18 +0800,Icenowy Zheng写道:
> 在 2018-03-08四的 12:55 +0100,Marc Gonzalez写道:
> > On 08/03/2018 06:48, Icenowy Zheng wrote:
> > 
> > > I'm trying to implement a driver for the quirky (DW) PCIe RC in
> > > the
> > > Allwinner H6 SoC.
> > > 
> > > The quirk is that only the "dbi" space is always mapped, but at
> > > the 
> > > same time only 64KiB of other spaces (config, downstream IO and
> > > non- 
> > > prefetchable memory) are accessible. To access a certain address
> > > the 
> > > high 16-bit of the address (all bus addresses in H6 SoC are 32-
> > > bit 
> > > despite the CPU is 64-bit) needs to be written into the 
> > > PCIE_ADDR_PAGE_CFG register (a vendor-defined register in DBI 
> > > space). So the access to these spaces cannot be processed
> > > correctly 
> > > with just readl/writel, as the existing code does.
> > > 
> > > Is it possible to workaround this in the PCI subsystem of Linux?
> > 
> > I didn't think anyone would come up with something more broken
> > than tango's PCIe host bridge...
> > 
> > It's hard to understand what's going on inside the minds of these
> > HW devs. I mean, if the steps required are
> > 
> > WRITE MAGIC CONFIG REG
> > READ/WRITE ADDRESS REG
> > 
> > then the whole operation is clearly not atomic, and bad things
> > happen
> > when 2+ operations race.
> > 
> > Do they think in terms of single core systems with non-multitasking
> > OS?
> > Probably not.
> > 
> > Maybe they think it is not a problem to wrap the operation using a
> > mutex? I'll confess I have no idea how bad that is for performance.
> > However, that is not an option in Linux, because mem space accesses
> > are just plain mmio accesses, and it's not possible to rewrite the
> > drivers, even as an out-of-tree patch.
> > 
> > I suppose it could be possible to make the first write "magic" in
> > the
> > sense that it could "lock" the bus until the second access is
> > performed?
> > Sort of like an implicit HW mutex. Actually, tango has explicit HW
> > mutexes that work along these lines.
> 
> Unfortunately sun50i-h6 doesn't have it.
> 
> > 
> > > (I have thought a workaround that only maps the current
> > > accessible 
> > > 64KiB with the MMU, and when accessing the non-accessible part, 
> > > catch the page fault and re-setup the map to the new 64KiB page.
> > > But 
> > > surely it will kill the performance.)
> > 
> > The implementation might be non-trivial, as well.
> 
> It might be possible for set up a hypervisor to do it. (Although it's
> surely an abuse of the HYP/EL2 mode)
> 
> > 
> > > [tango is] still less quirky than Allwinner H6 PCIe. It's only a
> > > config/MMIO mux on tango; however on H6 PCIe both config space
> > > and
> > > MMIO space are splitted to many pages. So on H6 if no solution is
> > > worked out, it will not be unreliable -- it will be unusable
> > > instead.
> > 
> > I have only limited experience, but it seems that many useful PCIe
> > adapters require only very little mem address space.
> 
> But there's also many that require bigger memory address space than
> 64K.
> 
> e.g. the Atheros AR9462 802.11n adapter needs 512K non-prefetchable
> memory.
> 
> > 
> > For example, the USB3 PCIe adapter I tested my implementation with
> > requires only 8 KB.
> > 
> > 01:00.0 USB controller: Renesas Technology Corp. uPD720201 USB 3.0
> > Host Controller (rev 03) (prog-if 30 [XHCI])
> > Flags: fast devsel
> > Memory at 5040 (64-bit, non-prefetchable) [size=8K]
> > Capabilities: [50] Power Management version 3
> > Capabilities: [70] MSI: Enable- Count=1/8 Maskable- 64bit+
> > Capabilities: [90] MSI-X: Enable- Count=8 Masked-
> > Capabilities: [a0] Express Endpoint, MSI 00
> > Capabilities: [100] Advanced Error Reporting
> > Capabilities: [150] Latency Tolerance Reporting
> > 
> > 
> > Same thing for the following WiFi adapter.
> > 
> > 01:00.0 Network controller: Intel Corporation Wireless 7260 (rev
> > bb)
> > Subsystem: Intel Corporation Dual Band Wireless-AC 7260
> > Flags: fast devsel, IRQ 22
> > Memory at 5040 (64-bit, non-prefetchable) [disabled]
> > [size=8K]
> > Capabilities: [c8] Power Management version 3
> > Capabilities: [d0] MSI: Enable- Count=1/1 Maskable- 64bit+
> > Capabilities: [40] Express Endpoint, MSI 00
> > Capabilities: [100] Advanced Error Reporting
> > Capabilities: [140] Device Serial Number 48-51-b7-ff-ff-84-
> > 69-26
> > Capabilities: [14c] Latency Tolerance Reporting
> > Capabilities: [154] Vendor Specific Information: ID=cafe
> > Rev=1 Len=014 
> > 
> > 
> > Maybe you can decide that you will support only 64 KB?
> 
> Despite support of some devices will fail, I still think this is a
> good
> idea. It makes the problem much simpler -- it will become a mux among
> config space, IO space and 64KB non-prefetchable memory.

However the DW PCIe RC core claims 1MiB non-prefetchable 

Re: [linux-sunxi] Re: DRM fb issue - Kernel 4.15.7

2018-03-08 Thread @lex
Ok, will test  Ondřej and your branch. This can take some time, I think 
this weekend I can build and test both.

But this raised some questions:
a) How is this going to help you guys fix the stable 4.15.y ?
b) I applied the significant changes on DRM and sun4i (that i think would 
fix) on my 4.15 based on Ondřej 4.16 branch without much help, still same 
problem.
clk.c (4.16) has received some changes and I have not touched it.

[   58.647727] [ cut here ]
>
> [   58.647802] WARNING: CPU: 1 PID: 72 at 
>> drivers/gpu/drm/sun4i/sun4i_crtc.c:57 sun4i_crtc_atomic_begin+0x6c/0x70
>
> [   58.647814] Modules linked in: ir_lirc_codec lirc_dev sunxi_cir 
>> brcmfmac brcmutil g_serial ipv6
>
> [   58.647909] CPU: 1 PID: 72 Comm: irq/46-1ee. Not tainted 
>> 4.15.7-h2-2 #10
>
> [   58.647920] Hardware name: Allwinner sun8i Family
>
> [   58.647999] [] (unwind_backtrace) from [] 
>> (show_stack+0x10/0x14)
>
> [   58.648050] [] (show_stack) from [] 
>> (dump_stack+0x88/0x9c)
>
> [   58.648092] [] (dump_stack) from [] 
>> (__warn+0xdc/0xf4)
>
> [   58.648128] [] (__warn) from [] 
>> (warn_slowpath_null+0x40/0x48)
>
> [   58.648164] [] (warn_slowpath_null) from [] 
>> (sun4i_crtc_atomic_begin+0x6c/0x70)
>
> [   58.648215] [] (sun4i_crtc_atomic_begin) from [] 
>> (drm_atomic_helper_commit_planes+0x74/0x2ac)
>
> [   58.648264] [] (drm_atomic_helper_commit_planes) from 
>> [] (drm_atomic_helper_commit_tail+0x28/0x64)
>
> [   58.648311] [] (drm_atomic_helper_commit_tail) from 
>> [] (commit_tail+0x80/0x84)
>
> [   58.648355] [] (commit_tail) from [] 
>> (drm_atomic_helper_commit+0x120/0x124)
>
> [   58.648399] [] (drm_atomic_helper_commit) from [] 
>> (restore_fbdev_mode_atomic+0x178/0x1d4)
>
> [   58.648442] [] (restore_fbdev_mode_atomic) from [] 
>> (drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0)
>
> [   58.648482] [] (drm_fb_helper_restore_fbdev_mode_unlocked) 
>> from [] (drm_fb_helper_set_par+0x30/0x60)
>
> [   58.648521] [] (drm_fb_helper_set_par) from [] 
>> (drm_fb_helper_hotplug_event.part.11+0xa8/0xb0)
>
> [   58.648562] [] (drm_fb_helper_hotplug_event.part.11) from 
>> [] (drm_helper_hpd_irq_event+0xf0/0xfc)
>
> [   58.648609] [] (drm_helper_hpd_irq_event) from [] 
>> (dw_hdmi_irq+0xfc/0x184)
>
> [   58.648655] [] (dw_hdmi_irq) from [] 
>> (irq_thread_fn+0x1c/0x54)
>
> [   58.648692] [] (irq_thread_fn) from [] 
>> (irq_thread+0x138/0x1f8)
>
> [   58.648732] [] (irq_thread) from [] 
>> (kthread+0x148/0x150)
>
> [   58.648777] [] (kthread) from [] 
>> (ret_from_fork+0x14/0x2c)
>
> [   58.648794] ---[ end trace db8b21de0cfe2bd6 ]---
>
> [   58.904890] [ cut here ]
>
> [   58.905010] WARNING: CPU: 1 PID: 72 at 
>> drivers/gpu/drm/drm_vblank.c:1015 
>> drm_atomic_helper_wait_for_vblanks.part.1+0x10c/0x290
>
> [   58.905030] Modules linked in: ir_lirc_codec lirc_dev sunxi_cir 
>> brcmfmac brcmutil g_serial ipv6
>
> [   58.905207] CPU: 1 PID: 72 Comm: irq/46-1ee. Tainted: GW
>> 4.15.7-h2-2 #10
>
> [   58.905229] Hardware name: Allwinner sun8i Family
>
> [   58.905346] [] (unwind_backtrace) from [] 
>> (show_stack+0x10/0x14)
>
> [   58.905439] [] (show_stack) from [] 
>> (dump_stack+0x88/0x9c)
>
> [   58.905515] [] (dump_stack) from [] 
>> (__warn+0xdc/0xf4)
>
> [   58.905583] [] (__warn) from [] 
>> (warn_slowpath_null+0x40/0x48)
>
> [   58.905668] [] (warn_slowpath_null) from [] 
>> (drm_atomic_helper_wait_for_vblanks.part.1+0x10c/0x290)
>
> [   58.905770] [] (drm_atomic_helper_wait_for_vblanks.part.1) 
>> from [] (drm_atomic_helper_commit_tail+0x54/0x64)
>
> [   58.905860] [] (drm_atomic_helper_commit_tail) from 
>> [] (commit_tail+0x80/0x84)
>
> [   58.905950] [] (commit_tail) from [] 
>> (drm_atomic_helper_commit+0x120/0x124)
>
> [   58.906034] [] (drm_atomic_helper_commit) from [] 
>> (restore_fbdev_mode_atomic+0x178/0x1d4)
>
> [   58.906116] [] (restore_fbdev_mode_atomic) from [] 
>> (drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0)
>
> [   58.906195] [] (drm_fb_helper_restore_fbdev_mode_unlocked) 
>> from [] (drm_fb_helper_set_par+0x30/0x60)
>
> [   58.906273] [] (drm_fb_helper_set_par) from [] 
>> (drm_fb_helper_hotplug_event.part.11+0xa8/0xb0)
>
> [   58.906353] [] (drm_fb_helper_hotplug_event.part.11) from 
>> [] (drm_helper_hpd_irq_event+0xf0/0xfc)
>
> [   58.906445] [] (drm_helper_hpd_irq_event) from [] 
>> (dw_hdmi_irq+0xfc/0x184)
>
> [   58.906530] [] (dw_hdmi_irq) from [] 
>> (irq_thread_fn+0x1c/0x54)
>
> [   58.906601] [] (irq_thread_fn) from [] 
>> (irq_thread+0x138/0x1f8)
>
> [   58.906674] [] (irq_thread) from [] 
>> (kthread+0x148/0x150)
>
> [   58.906756] [] (kthread) from [] 
>> (ret_from_fork+0x14/0x2c)
>
> [   58.906789] ---[ end trace db8b21de0cfe2bd7 ]---
>
>
>
c) No one experienced this?

BR,
@lex



On Thursday, March 8, 2018 at 3:31:59 AM UTC-3, Jernej Škrabec wrote:
>
> Hi, 
>
> Dne sreda, 07. marec 2018 ob 23:48:30 CET je 'Ondřej Jirman' via 
> linux-sunxi 
> napisal(a): 
> > Hi, 
> > 
> > On Wed, Mar 07, 

[linux-sunxi] Re: Question about a quirky (DesignWare) PCIe RC in Allwinner H6

2018-03-08 Thread Icenowy Zheng
在 2018-03-08四的 12:55 +0100,Marc Gonzalez写道:
> On 08/03/2018 06:48, Icenowy Zheng wrote:
> 
> > I'm trying to implement a driver for the quirky (DW) PCIe RC in the
> > Allwinner H6 SoC.
> > 
> > The quirk is that only the "dbi" space is always mapped, but at
> > the 
> > same time only 64KiB of other spaces (config, downstream IO and
> > non- 
> > prefetchable memory) are accessible. To access a certain address
> > the 
> > high 16-bit of the address (all bus addresses in H6 SoC are 32-
> > bit 
> > despite the CPU is 64-bit) needs to be written into the 
> > PCIE_ADDR_PAGE_CFG register (a vendor-defined register in DBI 
> > space). So the access to these spaces cannot be processed
> > correctly 
> > with just readl/writel, as the existing code does.
> > 
> > Is it possible to workaround this in the PCI subsystem of Linux?
> 
> I didn't think anyone would come up with something more broken
> than tango's PCIe host bridge...
> 
> It's hard to understand what's going on inside the minds of these
> HW devs. I mean, if the steps required are
> 
>   WRITE MAGIC CONFIG REG
>   READ/WRITE ADDRESS REG
> 
> then the whole operation is clearly not atomic, and bad things happen
> when 2+ operations race.
> 
> Do they think in terms of single core systems with non-multitasking
> OS?
> Probably not.
> 
> Maybe they think it is not a problem to wrap the operation using a
> mutex? I'll confess I have no idea how bad that is for performance.
> However, that is not an option in Linux, because mem space accesses
> are just plain mmio accesses, and it's not possible to rewrite the
> drivers, even as an out-of-tree patch.
> 
> I suppose it could be possible to make the first write "magic" in the
> sense that it could "lock" the bus until the second access is
> performed?
> Sort of like an implicit HW mutex. Actually, tango has explicit HW
> mutexes that work along these lines.

Unfortunately sun50i-h6 doesn't have it.

> 
> > (I have thought a workaround that only maps the current accessible 
> > 64KiB with the MMU, and when accessing the non-accessible part, 
> > catch the page fault and re-setup the map to the new 64KiB page.
> > But 
> > surely it will kill the performance.)
> 
> The implementation might be non-trivial, as well.

It might be possible for set up a hypervisor to do it. (Although it's
surely an abuse of the HYP/EL2 mode)

> 
> > [tango is] still less quirky than Allwinner H6 PCIe. It's only a
> > config/MMIO mux on tango; however on H6 PCIe both config space and
> > MMIO space are splitted to many pages. So on H6 if no solution is
> > worked out, it will not be unreliable -- it will be unusable
> > instead.
> 
> I have only limited experience, but it seems that many useful PCIe
> adapters require only very little mem address space.

But there's also many that require bigger memory address space than
64K.

e.g. the Atheros AR9462 802.11n adapter needs 512K non-prefetchable
memory.

> 
> For example, the USB3 PCIe adapter I tested my implementation with
> requires only 8 KB.
> 
> 01:00.0 USB controller: Renesas Technology Corp. uPD720201 USB 3.0
> Host Controller (rev 03) (prog-if 30 [XHCI])
> Flags: fast devsel
> Memory at 5040 (64-bit, non-prefetchable) [size=8K]
> Capabilities: [50] Power Management version 3
> Capabilities: [70] MSI: Enable- Count=1/8 Maskable- 64bit+
> Capabilities: [90] MSI-X: Enable- Count=8 Masked-
> Capabilities: [a0] Express Endpoint, MSI 00
> Capabilities: [100] Advanced Error Reporting
> Capabilities: [150] Latency Tolerance Reporting
> 
> 
> Same thing for the following WiFi adapter.
> 
> 01:00.0 Network controller: Intel Corporation Wireless 7260 (rev bb)
> Subsystem: Intel Corporation Dual Band Wireless-AC 7260
> Flags: fast devsel, IRQ 22
> Memory at 5040 (64-bit, non-prefetchable) [disabled]
> [size=8K]
> Capabilities: [c8] Power Management version 3
> Capabilities: [d0] MSI: Enable- Count=1/1 Maskable- 64bit+
> Capabilities: [40] Express Endpoint, MSI 00
> Capabilities: [100] Advanced Error Reporting
> Capabilities: [140] Device Serial Number 48-51-b7-ff-ff-84-
> 69-26
> Capabilities: [14c] Latency Tolerance Reporting
> Capabilities: [154] Vendor Specific Information: ID=cafe
> Rev=1 Len=014 
> 
> 
> Maybe you can decide that you will support only 64 KB?

Despite support of some devices will fail, I still think this is a good
idea. It makes the problem much simpler -- it will become a mux among
config space, IO space and 64KB non-prefetchable memory.

If a PCIe-fixing hypervisor is done it can simply skip the kernel glue,
but let the kernel configure it with ECAM mode.

> 
> Regards.
> 

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more 

[linux-sunxi] Re: [PATCH 4/5] dt-bindings: pwm: sunxi: add new compatible strings

2018-03-08 Thread Andre Przywara
Hi,

On 08/03/18 02:08, Rob Herring wrote:
> On Wed, Mar 07, 2018 at 02:07:18AM +, Andre Przywara wrote:
>> The PWM controllers found in the Allwinner A64, H5 and H6 SoCs are fully
>> compatible to the PWM controllers found in the A13 and H3.
> 
> If fully compatible, then shouldn't there be a fallback to one of the 
> existing compatible strings?

Yes, that was the idea. I was already wondering how we would
differentiate that from "real" compatible strings, but couldn't find
convincing examples. So should that read:

- compatible: should at least contain be one of:
- "allwinner,sun4i-a10-pwm"
- "allwinner,sun5i-a10s-pwm"
- "allwinner,sun5i-a13-pwm"
- "allwinner,sun7i-a20-pwm"
- "allwinner,sun8i-h3-pwm"
  May in addition contain one of:
- "allwinner,sun50i-a64-pwm"
- "allwinner,sun50i-h5-pwm"
- "allwinner,sun50i-h6-pwm"

And this would possibly need to be amended once we need to actually use
one of those strings (to implement a quirk, for instance)?
Or is there some other way of reserving those compatible strings, which
are not expected to be matched in drivers directly?

Cheers,
Andre.

>> Add new compatible strings for those SoCs to the binding document, so
>> that they can be safely used, together with a fallback string
>> (preferably "allwinner,sun5i-a13-pwm").
>> Add add the optionals "resets" property, needed by the H6.
>>
>> Signed-off-by: Andre Przywara 
>> ---
>>  Documentation/devicetree/bindings/pwm/pwm-sun4i.txt | 6 ++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt 
>> b/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
>> index 51ff54c8b8ef..b3a127a0e58c 100644
>> --- a/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
>> +++ b/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
>> @@ -7,11 +7,17 @@ Required properties:
>>  - "allwinner,sun5i-a13-pwm"
>>  - "allwinner,sun7i-a20-pwm"
>>  - "allwinner,sun8i-h3-pwm"
>> +- "allwinner,sun50i-a64-pwm"
>> +- "allwinner,sun50i-h5-pwm"
>> +- "allwinner,sun50i-h6-pwm"
>>- reg: physical base address and length of the controller's registers
>>- #pwm-cells: should be 3. See pwm.txt in this directory for a 
>> description of
>>  the cells format.
>>- clocks: From common clock binding, handle to the parent clock.
>>  
>> +Optional properties:
>> +  - resets: shall be the reset control phandle for the PWM block, if 
>> required.
>> +
>>  Example:
>>  
>>  pwm: pwm@1c20e00 {
>> -- 
>> 2.14.1
>>

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.