Re: [Intel-gfx] [PATCH] drm/i915/dsi: Add PORT_TX_DW7 programming to DSI vswing sequence

2018-12-15 Thread kbuild test robot
Hi Clint,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20181214]
[cannot apply to v4.20-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/clinton-a-taylor-intel-com/drm-i915-dsi-Add-PORT_TX_DW7-programming-to-DSI-vswing-sequence/20181215-101421
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/intel_drv.h:33:0,
from drivers/gpu/drm/i915/intel_dsi.h:30,
from drivers/gpu/drm/i915/icl_dsi.c:30:
   drivers/gpu/drm/i915/icl_dsi.c: In function 
'dsi_program_swing_and_deemphasis':
   drivers/gpu/drm/i915/icl_dsi.c:239:19: error: implicit declaration of 
function 'ICL_PORT_TX_DW7_LN0'; did you mean 'ICL_PORT_TX_DW4_LN0'? 
[-Werror=implicit-function-declaration]
  tmp = I915_READ(ICL_PORT_TX_DW7_LN0(port));
  ^
   drivers/gpu/drm/i915/i915_drv.h:3452:70: note: in definition of macro 
'I915_READ'
#define I915_READ(reg)  dev_priv->uncore.funcs.mmio_readl(dev_priv, (reg), 
true)
 ^~~
   drivers/gpu/drm/i915/i915_drv.h:3452:69: error: incompatible type for 
argument 2 of 'dev_priv->uncore.funcs.mmio_readl'
#define I915_READ(reg)  dev_priv->uncore.funcs.mmio_readl(dev_priv, (reg), 
true)
^
   drivers/gpu/drm/i915/icl_dsi.c:239:9: note: in expansion of macro 'I915_READ'
  tmp = I915_READ(ICL_PORT_TX_DW7_LN0(port));
^
   drivers/gpu/drm/i915/i915_drv.h:3452:69: note: expected 'i915_reg_t {aka 
struct }' but argument is of type 'int'
#define I915_READ(reg)  dev_priv->uncore.funcs.mmio_readl(dev_priv, (reg), 
true)
^
   drivers/gpu/drm/i915/icl_dsi.c:239:9: note: in expansion of macro 'I915_READ'
  tmp = I915_READ(ICL_PORT_TX_DW7_LN0(port));
^
>> drivers/gpu/drm/i915/icl_dsi.c:242:14: error: implicit declaration of 
>> function 'ICL_PORT_TX_DW7_GRP'; did you mean 'ICL_PORT_TX_DW4_GRP'? 
>> [-Werror=implicit-function-declaration]
  I915_WRITE(ICL_PORT_TX_DW7_GRP(port), tmp);
 ^
   drivers/gpu/drm/i915/i915_drv.h:3453:76: note: in definition of macro 
'I915_WRITE'
#define I915_WRITE(reg, val) dev_priv->uncore.funcs.mmio_writel(dev_priv, 
(reg), (val), true)
   
^~~
   drivers/gpu/drm/i915/i915_drv.h:3453:75: error: incompatible type for 
argument 2 of 'dev_priv->uncore.funcs.mmio_writel'
#define I915_WRITE(reg, val) dev_priv->uncore.funcs.mmio_writel(dev_priv, 
(reg), (val), true)
  ^
   drivers/gpu/drm/i915/icl_dsi.c:242:3: note: in expansion of macro 
'I915_WRITE'
  I915_WRITE(ICL_PORT_TX_DW7_GRP(port), tmp);
  ^~
   drivers/gpu/drm/i915/i915_drv.h:3453:75: note: expected 'i915_reg_t {aka 
struct }' but argument is of type 'int'
#define I915_WRITE(reg, val) dev_priv->uncore.funcs.mmio_writel(dev_priv, 
(reg), (val), true)
  ^
   drivers/gpu/drm/i915/icl_dsi.c:242:3: note: in expansion of macro 
'I915_WRITE'
  I915_WRITE(ICL_PORT_TX_DW7_GRP(port), tmp);
  ^~
   drivers/gpu/drm/i915/icl_dsi.c:244:19: error: implicit declaration of 
function 'ICL_PORT_TX_DW7_AUX'; did you mean 'ICL_PORT_TX_DW2_AUX'? 
[-Werror=implicit-function-declaration]
  tmp = I915_READ(ICL_PORT_TX_DW7_AUX(port));
  ^
   drivers/gpu/drm/i915/i915_drv.h:3452:70: note: in definition of macro 
'I915_READ'
#define I915_READ(reg)  dev_priv->uncore.funcs.mmio_readl(dev_priv, (reg), 
true)
 ^~~
   drivers/gpu/drm/i915/i915_drv.h:3452:69: error: incompatible type for 
argument 2 of 'dev_priv->uncore.funcs.mmio_readl'
#define I915_READ(reg)  dev_priv->uncore.funcs.mmio_readl(dev_priv, (reg), 
true)
^
   drivers/gpu/drm/i915/icl_dsi.c:244:9: note: in expansion of macro 'I915_READ&#x

Re: [Intel-gfx] [PATCH v9 35/39] misc/mei/hdcp: Component framework for I915 Interface

2018-12-15 Thread Winkler, Tomas
> 
> On Thu, Dec 13, 2018 at 5:27 PM Winkler, Tomas 
> wrote:
> >
> > > On Thu, Dec 13, 2018 at 1:36 PM C, Ramalingam
> > > 
> > > wrote:
> > > >
> > > > Tomas and Daniel,
> > > >
> > > > We got an issue here.
> > > >
> > > > The relationship that we try to build between I915 and mei_hdcp is as
> follows:
> > > >
> > > > We are using the components to establish the relationship.
> > > > I915 is component master where as mei_hdcp is component.
> > > > I915 adds the component master during the module load. mei_hdcp
> > > > adds the
> > > component when the driver->probe is called (on device driver binding).
> > > > I915 forces itself such that until mei_hdcp component is added
> > > > I915_load
> > > wont be complete.
> > > > Similarly on complete system, if mei_hdcp component is removed,
> > > immediately I915 unregister itself and HW will be shutdown.
> > > >
> > > > This is completely fine when the modules are loaded and unloaded.
> > > >
> > > > But during suspend, mei device disappears and mei bus handles it
> > > > by
> > > unbinding device and driver by calling driver->remove.
> > > > This in-turn removes the component and triggers the master unbind
> > > > of I915
> > > where, I915 unregister itself.
> > > > This cause the HW state mismatch during the suspend and resume.
> > > >
> > > > Please check the powerwell mismatch errors at CI report for v9
> > > > https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_3412/fi-glk-j4005/
> > > > igt@
> > > > gem_exec_susp...@basic-s3.html
> > > >
> > > > More over unregistering I915 during the suspend is not expected.
> > > > So how do
> > > we handle this?
> > >
> > > Bit more context from our irc discussion with Ram:
> > >
> > > I found this very surprising, since I don't know of any other
> > > subsystems where the devices get outright removed when going through a
> suspend/resume cycle.
> > > The device model was built to handle this stuff
> > > correctly: First clients/devices/interfaces get suspend, then the
> > > parent/bridge/bus. Same dance in reverse when resuming. This even
> > > holds for lots of hotpluggable buses, where child devices could
> > > indeed disappear on resume, but as long as they don't, everything
> > > stays the same. It's really surprising for something that's soldered onto 
> > > the
> board like ME.
> >
> > HDCP is an application in the ME it's not ME itself..  On the linux
> > side HDCP2 is a virtual device  on mei client virtual bus, the bus  is 
> > teared
> down on ME reset, which mostly happen  on power transitions.
> > Theoretically,  we could keep it up during power transitions, but so
> > fare it was not necessary and second it's not guarantie that the all ME
> applications will reappear after reset.
> 
> When does this happen that an ME application doesn't come back after e.g.
> suspend/resume?
No, this can happen in special flows such as  fw updates and error conditions, 
but is has to be supported as well. 
 
> 
> Also, what's all the place where this reset can happen? Just
> suspend/resume/hibernate and all these, or also at other times?

Also on errors and fw update,  the basic assumption is here that it can happen 
any time. 

> How does userspace deal with the reset over s/r? I'm assuming that at least 
> the
> device node file will become invalid (or whatever you're using as userspace
> api), so if userspace is accessing stuff on the me at the same time as we do a
> suspend/resume, what happens?
> 
> > > Aside: We'll probably need a device_link to make sure mei_hdcp is
> > > fully resumed before i915 gets resumed, but that's kinda a detail for 
> > > later
> on.
> >
> > Frankly I don’t believe there is currently exact abstraction that
> > supports this model, neither components nor device_link .
> > So fare we used class interface for other purposes, it worked well.
> 
> I'm not clear on what class interface has to do with component or device link.
> They all solve different problems, at least as far as I understand all this 
> stuff ...
> -Daniel

It comes instead of it, device_link is mostly used for power management and 
component as we see know is not what we need as HDCP 
Is a b it volitle. 
class_interface  gives you two handlers: add and remove device, that's all what 
is needed for the current implementation. 
> 
> > > Tomas, can you pls explain why mei is designed like this? Or is
> > > there something else we're missing (I didn't dig through the mei bus
> > > in detail at all, so not clear on what exactly is going on there).
> > Above.
> > >
> > > Also pulling in device model and suspend/resume experts.
> > >
> > > Thanks, Daniel
> > >
> > > >
> > > > -Ram
> > > >
> > > > On 12/13/2018 9:31 AM, Ramalingam C wrote:
> > > >
> > > > Mei hdcp driver is designed as component slave for the I915
> > > > component master.
> > > >
> > > > v2: Rebased.
> > > > v3:
> > > >   Notifier chain is adopted for cldev state update [Tomas]
> > > > v4:
> > > >   Made static dummy functions as inline in mei_hdcp.h
> > > >   API for