[PATCH v6] drm: bridge/dw_hdmi: add dw hdmi i2c bus adapter support

2016-09-19 Thread Philipp Zabel
Am Freitag, den 16.09.2016, 20:38 +0300 schrieb Vladimir Zapolskiy:
> Hi Philipp,
[...]
> >> as far as I know David accepts pull requests from you, can you please
> >> create and send a pull request for v4.9 containing these changes?
> >>
> >> https://patchwork.kernel.org/patch/9284717/ -- with Russell's ack
> >
> > Is that a forward looking statement? I don't see Russell's ack.
> 
> Here it is: http://www.spinics.net/lists/dri-devel/msg115880.html

Thank you.

> Please don't bother about two other commits from that series, at least
> there are official maintainers for the changes. And by the way please
> consider to add youself as a maintainer of DW HDMI.
> 
> >> https://patchwork.kernel.org/patch/9296883/ -- with Rob's ack and 
> >> yours tested-by
> >>
> >> Some users anticipate this change, for example see 
> >> https://lkml.org/lkml/2016/9/14/55
> >
> > Those I see. I can re-test and prepare a pull request.

Re-tested on Nitrogen6X with this patch applied:

--8<--
diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi 
b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
index 97d9c33..5d60c14 100644
--- a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
@@ -292,7 +292,6 @@
 };

  {
-   ddc-i2c-bus = <>;
status = "okay";
 };

@@ -439,8 +438,8 @@

pinctrl_i2c2: i2c2grp {
fsl,pins = <
-   MX6QDL_PAD_KEY_COL3__I2C2_SCL   
0x4001b8b1
-   MX6QDL_PAD_KEY_ROW3__I2C2_SDA   
0x4001b8b1
+   MX6QDL_PAD_KEY_COL3__HDMI_TX_DDC_SCL
0x4001b8b1
+   MX6QDL_PAD_KEY_ROW3__HDMI_TX_DDC_SDA
0x4001b8b1
>;
};

-->8--

I'll send a pull request shortly.

regards
Philipp



[PATCH v6] drm: bridge/dw_hdmi: add dw hdmi i2c bus adapter support

2016-09-16 Thread Vladimir Zapolskiy
Hi Philipp,

On 09/16/2016 06:21 PM, Philipp Zabel wrote:
> Hi Vladimir,
>
> Am Mittwoch, den 14.09.2016, 21:46 +0300 schrieb Vladimir Zapolskiy:
>> Hi Philipp,
>>
>> On 08/29/2016 06:50 PM, Rob Herring wrote:
>>> On Wed, Aug 24, 2016 at 08:46:37AM +0300, Vladimir Zapolskiy wrote:
 The change adds support of internal HDMI I2C master controller, this
 subdevice is used by default, if "ddc-i2c-bus" DT property is omitted.

 The main purpose of this functionality is to support reading EDID from
 an HDMI monitor on boards, which don't have an I2C bus connected to
 DDC pins.

 The current implementation does not support "I2C Master Interface
 Extended Read Mode" to read data addressed by non-zero segment
 pointer, this means that if EDID has more than 1 extension blocks,
 EDID reading operation won't succeed, in my practice all tested HDMI
 monitors have at maximum one extension block.

 Signed-off-by: Vladimir Zapolskiy 
 ---
 The change is based on top of v4.8.0-rc1 and a fix in DW HDMI driver
 https://patchwork.kernel.org/patch/9284717/

 Changes from v5 to v6:
 * rebased on top of v4.8.0-rc1
 * fixed one improper resource deallocation on error path of dw_hdmi_bind()
 * added a comment describing a mutex asked by checkpatch.pl --strict

 Link to version v5: https://patchwork.kernel.org/patch/7279831/

 Changes from v4 to v5:
 * do I2C bus controller initialization only once in bind() as it was done
   in v1-v3 of the change.

 Changes from v3 to v4, thanks to Doug and Philipp for review:
 * set speed mode after software reset in dw_hdmi_i2c_init()
 * by default set standard speed mode instead of fast speed mode, on iMX6Q
   this configures SCL to 100 KHz, which is compliant with HDMI 1.3a spec
 * do I2C bus controller reinitialization on every data transfer,
   this change hopefully solves some observed problems on RK3288 platform
 * added short functional change description to dw_hdmi.txt

 v3 of the change was

   Tested-by: Philipp Zabel 

 Changes from v2 to v3, thanks to Russell:
 * moved register field value definitions to dw_hdmi.h
 * made completions uninterruptible to avoid transfer retries if interrupted
 * use one completion for both read and write transfers as in v1,
   operation_reg is removed
 * redundant i2c->stat = 0 is removed from dw_hdmi_i2c_read/write()
 * struct i2c_algorithm dw_hdmi_algorithm is qualified as const
 * dw_hdmi_i2c_adapter() does not modify struct dw_hdmi on error path
 * dw_hdmi_i2c_irq() does not modify hdmi->i2c->stat, if interrupt is
   not for I2CM
 * spin lock is removed from dw_hdmi_i2c_irq()
 * removed spin lock from dw_hdmi_i2c_xfer() around write to
   HDMI_IH_MUTE_I2CM_STAT0 register
 * split loop over message array in dw_hdmi_i2c_xfer() to validation
   and transfer parts
 * added a mutex to serialize I2C transfer requests, i2c->lock is
   completely removed
 * removed if(len) check from dw_hdmi_i2c_write(), hardware supports
   only len>0 transfers
 * described extension blocks <= 1 limitation in the commit message
 * a number of minor clean ups

 Changes from v1 to v2:
 * fixed a devm_kfree() signature
 * split completions for read and write operations

  .../devicetree/bindings/display/bridge/dw_hdmi.txt |   4 +-
>>>
>>> Acked-by: Rob Herring 
>>>
  drivers/gpu/drm/bridge/dw-hdmi.c   | 265 
 -
  drivers/gpu/drm/bridge/dw-hdmi.h   |  19 ++
  3 files changed, 281 insertions(+), 7 deletions(-)
>>
>> as far as I know David accepts pull requests from you, can you please
>> create and send a pull request for v4.9 containing these changes?
>>
>> https://patchwork.kernel.org/patch/9284717/ -- with Russell's ack
>
> Is that a forward looking statement? I don't see Russell's ack.

Here it is: http://www.spinics.net/lists/dri-devel/msg115880.html

Please don't bother about two other commits from that series, at least
there are official maintainers for the changes. And by the way please
consider to add youself as a maintainer of DW HDMI.

>> https://patchwork.kernel.org/patch/9296883/ -- with Rob's ack and yours 
>> tested-by
>>
>> Some users anticipate this change, for example see 
>> https://lkml.org/lkml/2016/9/14/55
>
> Those I see. I can re-test and prepare a pull request.
>

Thank you in advance!

--
With best wishes,
Vladimir


[PATCH v6] drm: bridge/dw_hdmi: add dw hdmi i2c bus adapter support

2016-09-16 Thread Philipp Zabel
Hi Vladimir,

Am Mittwoch, den 14.09.2016, 21:46 +0300 schrieb Vladimir Zapolskiy:
> Hi Philipp,
> 
> On 08/29/2016 06:50 PM, Rob Herring wrote:
> > On Wed, Aug 24, 2016 at 08:46:37AM +0300, Vladimir Zapolskiy wrote:
> >> The change adds support of internal HDMI I2C master controller, this
> >> subdevice is used by default, if "ddc-i2c-bus" DT property is omitted.
> >>
> >> The main purpose of this functionality is to support reading EDID from
> >> an HDMI monitor on boards, which don't have an I2C bus connected to
> >> DDC pins.
> >>
> >> The current implementation does not support "I2C Master Interface
> >> Extended Read Mode" to read data addressed by non-zero segment
> >> pointer, this means that if EDID has more than 1 extension blocks,
> >> EDID reading operation won't succeed, in my practice all tested HDMI
> >> monitors have at maximum one extension block.
> >>
> >> Signed-off-by: Vladimir Zapolskiy 
> >> ---
> >> The change is based on top of v4.8.0-rc1 and a fix in DW HDMI driver
> >> https://patchwork.kernel.org/patch/9284717/
> >>
> >> Changes from v5 to v6:
> >> * rebased on top of v4.8.0-rc1
> >> * fixed one improper resource deallocation on error path of dw_hdmi_bind()
> >> * added a comment describing a mutex asked by checkpatch.pl --strict
> >>
> >> Link to version v5: https://patchwork.kernel.org/patch/7279831/
> >>
> >> Changes from v4 to v5:
> >> * do I2C bus controller initialization only once in bind() as it was done
> >>   in v1-v3 of the change.
> >>
> >> Changes from v3 to v4, thanks to Doug and Philipp for review:
> >> * set speed mode after software reset in dw_hdmi_i2c_init()
> >> * by default set standard speed mode instead of fast speed mode, on iMX6Q
> >>   this configures SCL to 100 KHz, which is compliant with HDMI 1.3a spec
> >> * do I2C bus controller reinitialization on every data transfer,
> >>   this change hopefully solves some observed problems on RK3288 platform
> >> * added short functional change description to dw_hdmi.txt
> >>
> >> v3 of the change was
> >>
> >>   Tested-by: Philipp Zabel 
> >>
> >> Changes from v2 to v3, thanks to Russell:
> >> * moved register field value definitions to dw_hdmi.h
> >> * made completions uninterruptible to avoid transfer retries if interrupted
> >> * use one completion for both read and write transfers as in v1,
> >>   operation_reg is removed
> >> * redundant i2c->stat = 0 is removed from dw_hdmi_i2c_read/write()
> >> * struct i2c_algorithm dw_hdmi_algorithm is qualified as const
> >> * dw_hdmi_i2c_adapter() does not modify struct dw_hdmi on error path
> >> * dw_hdmi_i2c_irq() does not modify hdmi->i2c->stat, if interrupt is
> >>   not for I2CM
> >> * spin lock is removed from dw_hdmi_i2c_irq()
> >> * removed spin lock from dw_hdmi_i2c_xfer() around write to
> >>   HDMI_IH_MUTE_I2CM_STAT0 register
> >> * split loop over message array in dw_hdmi_i2c_xfer() to validation
> >>   and transfer parts
> >> * added a mutex to serialize I2C transfer requests, i2c->lock is
> >>   completely removed
> >> * removed if(len) check from dw_hdmi_i2c_write(), hardware supports
> >>   only len>0 transfers
> >> * described extension blocks <= 1 limitation in the commit message
> >> * a number of minor clean ups
> >>
> >> Changes from v1 to v2:
> >> * fixed a devm_kfree() signature
> >> * split completions for read and write operations
> >>
> >>  .../devicetree/bindings/display/bridge/dw_hdmi.txt |   4 +-
> >
> > Acked-by: Rob Herring 
> >
> >>  drivers/gpu/drm/bridge/dw-hdmi.c   | 265 
> >> -
> >>  drivers/gpu/drm/bridge/dw-hdmi.h   |  19 ++
> >>  3 files changed, 281 insertions(+), 7 deletions(-)
> 
> as far as I know David accepts pull requests from you, can you please
> create and send a pull request for v4.9 containing these changes?
> 
> https://patchwork.kernel.org/patch/9284717/ -- with Russell's ack

Is that a forward looking statement? I don't see Russell's ack.

> https://patchwork.kernel.org/patch/9296883/ -- with Rob's ack and yours 
> tested-by
> 
> Some users anticipate this change, for example see 
> https://lkml.org/lkml/2016/9/14/55

Those I see. I can re-test and prepare a pull request.

regards
Philipp



[PATCH v6] drm: bridge/dw_hdmi: add dw hdmi i2c bus adapter support

2016-09-14 Thread Vladimir Zapolskiy
Hi Philipp,

On 08/29/2016 06:50 PM, Rob Herring wrote:
> On Wed, Aug 24, 2016 at 08:46:37AM +0300, Vladimir Zapolskiy wrote:
>> The change adds support of internal HDMI I2C master controller, this
>> subdevice is used by default, if "ddc-i2c-bus" DT property is omitted.
>>
>> The main purpose of this functionality is to support reading EDID from
>> an HDMI monitor on boards, which don't have an I2C bus connected to
>> DDC pins.
>>
>> The current implementation does not support "I2C Master Interface
>> Extended Read Mode" to read data addressed by non-zero segment
>> pointer, this means that if EDID has more than 1 extension blocks,
>> EDID reading operation won't succeed, in my practice all tested HDMI
>> monitors have at maximum one extension block.
>>
>> Signed-off-by: Vladimir Zapolskiy 
>> ---
>> The change is based on top of v4.8.0-rc1 and a fix in DW HDMI driver
>> https://patchwork.kernel.org/patch/9284717/
>>
>> Changes from v5 to v6:
>> * rebased on top of v4.8.0-rc1
>> * fixed one improper resource deallocation on error path of dw_hdmi_bind()
>> * added a comment describing a mutex asked by checkpatch.pl --strict
>>
>> Link to version v5: https://patchwork.kernel.org/patch/7279831/
>>
>> Changes from v4 to v5:
>> * do I2C bus controller initialization only once in bind() as it was done
>>   in v1-v3 of the change.
>>
>> Changes from v3 to v4, thanks to Doug and Philipp for review:
>> * set speed mode after software reset in dw_hdmi_i2c_init()
>> * by default set standard speed mode instead of fast speed mode, on iMX6Q
>>   this configures SCL to 100 KHz, which is compliant with HDMI 1.3a spec
>> * do I2C bus controller reinitialization on every data transfer,
>>   this change hopefully solves some observed problems on RK3288 platform
>> * added short functional change description to dw_hdmi.txt
>>
>> v3 of the change was
>>
>>   Tested-by: Philipp Zabel 
>>
>> Changes from v2 to v3, thanks to Russell:
>> * moved register field value definitions to dw_hdmi.h
>> * made completions uninterruptible to avoid transfer retries if interrupted
>> * use one completion for both read and write transfers as in v1,
>>   operation_reg is removed
>> * redundant i2c->stat = 0 is removed from dw_hdmi_i2c_read/write()
>> * struct i2c_algorithm dw_hdmi_algorithm is qualified as const
>> * dw_hdmi_i2c_adapter() does not modify struct dw_hdmi on error path
>> * dw_hdmi_i2c_irq() does not modify hdmi->i2c->stat, if interrupt is
>>   not for I2CM
>> * spin lock is removed from dw_hdmi_i2c_irq()
>> * removed spin lock from dw_hdmi_i2c_xfer() around write to
>>   HDMI_IH_MUTE_I2CM_STAT0 register
>> * split loop over message array in dw_hdmi_i2c_xfer() to validation
>>   and transfer parts
>> * added a mutex to serialize I2C transfer requests, i2c->lock is
>>   completely removed
>> * removed if(len) check from dw_hdmi_i2c_write(), hardware supports
>>   only len>0 transfers
>> * described extension blocks <= 1 limitation in the commit message
>> * a number of minor clean ups
>>
>> Changes from v1 to v2:
>> * fixed a devm_kfree() signature
>> * split completions for read and write operations
>>
>>  .../devicetree/bindings/display/bridge/dw_hdmi.txt |   4 +-
>
> Acked-by: Rob Herring 
>
>>  drivers/gpu/drm/bridge/dw-hdmi.c   | 265 
>> -
>>  drivers/gpu/drm/bridge/dw-hdmi.h   |  19 ++
>>  3 files changed, 281 insertions(+), 7 deletions(-)

as far as I know David accepts pull requests from you, can you please
create and send a pull request for v4.9 containing these changes?

https://patchwork.kernel.org/patch/9284717/ -- with Russell's ack
https://patchwork.kernel.org/patch/9296883/ -- with Rob's ack and yours 
tested-by

Some users anticipate this change, for example see 
https://lkml.org/lkml/2016/9/14/55

--
With best wishes,
Vladimir


[PATCH v6] drm: bridge/dw_hdmi: add dw hdmi i2c bus adapter support

2016-09-02 Thread Vladimir Zapolskiy
Hello Russell,

On 08/24/2016 08:46 AM, Vladimir Zapolskiy wrote:
> The change adds support of internal HDMI I2C master controller, this
> subdevice is used by default, if "ddc-i2c-bus" DT property is omitted.
>
> The main purpose of this functionality is to support reading EDID from
> an HDMI monitor on boards, which don't have an I2C bus connected to
> DDC pins.
>
> The current implementation does not support "I2C Master Interface
> Extended Read Mode" to read data addressed by non-zero segment
> pointer, this means that if EDID has more than 1 extension blocks,
> EDID reading operation won't succeed, in my practice all tested HDMI
> monitors have at maximum one extension block.
>
> Signed-off-by: Vladimir Zapolskiy 
> ---

do you object to this change?

--
With best wishes,
Vladimir


[PATCH v6] drm: bridge/dw_hdmi: add dw hdmi i2c bus adapter support

2016-08-30 Thread Emil Velikov
On 30 August 2016 at 09:12, Russell King - ARM Linux
 wrote:
> On Mon, Aug 29, 2016 at 05:41:10PM +0100, Emil Velikov wrote:
>> Hi all,
>>
>> On 24 August 2016 at 06:46, Vladimir Zapolskiy
>>  wrote:
>>
>> >  MODULE_AUTHOR("Sascha Hauer ");
>> >  MODULE_AUTHOR("Andy Yan ");
>> >  MODULE_AUTHOR("Yakir Yang ");
>> > +MODULE_AUTHOR("Vladimir Zapolskiy ");
>> Don't meant to start a flame-war or alike but to educate myself:
>> Where does one draw the line about adding new author(s) of said
>> module/subsystem ?
>>
>> Afaict this is the most common (?) driver in DRM where the list has
>> grown over time. Should we do the same with others ?
>
> ... and I'm responsible for just over half the commits in the mainline
> kernel and I haven't added myself.  I generally only add myself if I'm
> creating new code or been involved in adding new code, I don't add if
> I'm merely modifying existing code unless I've replaced a large
> quantity of the code.  I think the question people need to ask is:
>
>   "Have I contributed a significant set of changes to be able to claim
>shared authorship of that code?"
>
> You wouldn't claim authorship of a 500 page book if you suggested a
> few edits here and there.
>
> Looking at co-authorship in google, I came across:
>
> http://www.southernfriedscience.com/to-co-author-or-not-to-co-author/
>
> which has an interesting list of points on this subject, although more
> biased to research papers, which is where this problem normally arises.
> That seems to back up my idea of "significant contribution" not just
> a few minor changes.
>
> The question then becomes... what is a significant contribution. :)
>
Roughly my line of thinking as well.

Considering that the driver has been developed independently in one
shape or another for a few years I think it's perfectly reasonable in
this case.

Thanks for the input all !
Emil


[PATCH v6] drm: bridge/dw_hdmi: add dw hdmi i2c bus adapter support

2016-08-30 Thread Russell King - ARM Linux
On Mon, Aug 29, 2016 at 05:41:10PM +0100, Emil Velikov wrote:
> Hi all,
> 
> On 24 August 2016 at 06:46, Vladimir Zapolskiy
>  wrote:
> 
> >  MODULE_AUTHOR("Sascha Hauer ");
> >  MODULE_AUTHOR("Andy Yan ");
> >  MODULE_AUTHOR("Yakir Yang ");
> > +MODULE_AUTHOR("Vladimir Zapolskiy ");
> Don't meant to start a flame-war or alike but to educate myself:
> Where does one draw the line about adding new author(s) of said
> module/subsystem ?
> 
> Afaict this is the most common (?) driver in DRM where the list has
> grown over time. Should we do the same with others ?

... and I'm responsible for just over half the commits in the mainline
kernel and I haven't added myself.  I generally only add myself if I'm
creating new code or been involved in adding new code, I don't add if
I'm merely modifying existing code unless I've replaced a large
quantity of the code.  I think the question people need to ask is:

  "Have I contributed a significant set of changes to be able to claim
   shared authorship of that code?"

You wouldn't claim authorship of a 500 page book if you suggested a
few edits here and there.

Looking at co-authorship in google, I came across:

http://www.southernfriedscience.com/to-co-author-or-not-to-co-author/

which has an interesting list of points on this subject, although more
biased to research papers, which is where this problem normally arises.
That seems to back up my idea of "significant contribution" not just
a few minor changes.

The question then becomes... what is a significant contribution. :)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.


[PATCH v6] drm: bridge/dw_hdmi: add dw hdmi i2c bus adapter support

2016-08-29 Thread Vladimir Zapolskiy
Hi Emil,

On 08/29/2016 07:41 PM, Emil Velikov wrote:
> Hi all,
>
> On 24 August 2016 at 06:46, Vladimir Zapolskiy
>  wrote:
>
>>  MODULE_AUTHOR("Sascha Hauer ");
>>  MODULE_AUTHOR("Andy Yan ");
>>  MODULE_AUTHOR("Yakir Yang ");
>> +MODULE_AUTHOR("Vladimir Zapolskiy ");
> Don't meant to start a flame-war or alike but to educate myself:
> Where does one draw the line about adding new author(s) of said
> module/subsystem ?

I support you, let's don't sink. Since it evokes questions I'm
ready to provide you with more background information.

> Afaict this is the most common (?) driver in DRM where the list has
> grown over time. Should we do the same with others ?

If you look into the essense of this change it adds support to
an I2C master controller, which is a part of DW HDMI controller.

Originally this particular change was done as a separate driver
in I2C subsystem about 3 years ago and delivered to the customers,
about 2 years ago I published its RFC version:

  RFC:   https://patchwork.ozlabs.org/patch/405100/
  v1 discussion: http://www.mailbrowse.com/linux-i2c/20949.html

As you may see this was a stand-alone driver, which served as
the fourth I2C master controller on iMX6Q, however I believe
it is better to merge two drivers into one. Does it sound like
a good enough reason to merge lists of authors as well?

Hope it clarifies the situation a bit.

--
With best wishes,
Vladimir


[PATCH v6] drm: bridge/dw_hdmi: add dw hdmi i2c bus adapter support

2016-08-29 Thread Emil Velikov
Hi all,

On 24 August 2016 at 06:46, Vladimir Zapolskiy
 wrote:

>  MODULE_AUTHOR("Sascha Hauer ");
>  MODULE_AUTHOR("Andy Yan ");
>  MODULE_AUTHOR("Yakir Yang ");
> +MODULE_AUTHOR("Vladimir Zapolskiy ");
Don't meant to start a flame-war or alike but to educate myself:
Where does one draw the line about adding new author(s) of said
module/subsystem ?

Afaict this is the most common (?) driver in DRM where the list has
grown over time. Should we do the same with others ?

Thanks
Emil


[PATCH v6] drm: bridge/dw_hdmi: add dw hdmi i2c bus adapter support

2016-08-29 Thread Rob Herring
On Wed, Aug 24, 2016 at 08:46:37AM +0300, Vladimir Zapolskiy wrote:
> The change adds support of internal HDMI I2C master controller, this
> subdevice is used by default, if "ddc-i2c-bus" DT property is omitted.
> 
> The main purpose of this functionality is to support reading EDID from
> an HDMI monitor on boards, which don't have an I2C bus connected to
> DDC pins.
> 
> The current implementation does not support "I2C Master Interface
> Extended Read Mode" to read data addressed by non-zero segment
> pointer, this means that if EDID has more than 1 extension blocks,
> EDID reading operation won't succeed, in my practice all tested HDMI
> monitors have at maximum one extension block.
> 
> Signed-off-by: Vladimir Zapolskiy 
> ---
> The change is based on top of v4.8.0-rc1 and a fix in DW HDMI driver
> https://patchwork.kernel.org/patch/9284717/
> 
> Changes from v5 to v6:
> * rebased on top of v4.8.0-rc1
> * fixed one improper resource deallocation on error path of dw_hdmi_bind()
> * added a comment describing a mutex asked by checkpatch.pl --strict
> 
> Link to version v5: https://patchwork.kernel.org/patch/7279831/
> 
> Changes from v4 to v5:
> * do I2C bus controller initialization only once in bind() as it was done
>   in v1-v3 of the change.
> 
> Changes from v3 to v4, thanks to Doug and Philipp for review:
> * set speed mode after software reset in dw_hdmi_i2c_init()
> * by default set standard speed mode instead of fast speed mode, on iMX6Q
>   this configures SCL to 100 KHz, which is compliant with HDMI 1.3a spec
> * do I2C bus controller reinitialization on every data transfer,
>   this change hopefully solves some observed problems on RK3288 platform
> * added short functional change description to dw_hdmi.txt
> 
> v3 of the change was
> 
>   Tested-by: Philipp Zabel 
> 
> Changes from v2 to v3, thanks to Russell:
> * moved register field value definitions to dw_hdmi.h
> * made completions uninterruptible to avoid transfer retries if interrupted
> * use one completion for both read and write transfers as in v1,
>   operation_reg is removed
> * redundant i2c->stat = 0 is removed from dw_hdmi_i2c_read/write()
> * struct i2c_algorithm dw_hdmi_algorithm is qualified as const
> * dw_hdmi_i2c_adapter() does not modify struct dw_hdmi on error path
> * dw_hdmi_i2c_irq() does not modify hdmi->i2c->stat, if interrupt is
>   not for I2CM
> * spin lock is removed from dw_hdmi_i2c_irq()
> * removed spin lock from dw_hdmi_i2c_xfer() around write to
>   HDMI_IH_MUTE_I2CM_STAT0 register
> * split loop over message array in dw_hdmi_i2c_xfer() to validation
>   and transfer parts
> * added a mutex to serialize I2C transfer requests, i2c->lock is
>   completely removed
> * removed if(len) check from dw_hdmi_i2c_write(), hardware supports
>   only len>0 transfers
> * described extension blocks <= 1 limitation in the commit message
> * a number of minor clean ups
> 
> Changes from v1 to v2:
> * fixed a devm_kfree() signature
> * split completions for read and write operations
> 
>  .../devicetree/bindings/display/bridge/dw_hdmi.txt |   4 +-

Acked-by: Rob Herring 

>  drivers/gpu/drm/bridge/dw-hdmi.c   | 265 
> -
>  drivers/gpu/drm/bridge/dw-hdmi.h   |  19 ++
>  3 files changed, 281 insertions(+), 7 deletions(-)


[PATCH v6] drm: bridge/dw_hdmi: add dw hdmi i2c bus adapter support

2016-08-24 Thread Vladimir Zapolskiy
The change adds support of internal HDMI I2C master controller, this
subdevice is used by default, if "ddc-i2c-bus" DT property is omitted.

The main purpose of this functionality is to support reading EDID from
an HDMI monitor on boards, which don't have an I2C bus connected to
DDC pins.

The current implementation does not support "I2C Master Interface
Extended Read Mode" to read data addressed by non-zero segment
pointer, this means that if EDID has more than 1 extension blocks,
EDID reading operation won't succeed, in my practice all tested HDMI
monitors have at maximum one extension block.

Signed-off-by: Vladimir Zapolskiy 
---
The change is based on top of v4.8.0-rc1 and a fix in DW HDMI driver
https://patchwork.kernel.org/patch/9284717/

Changes from v5 to v6:
* rebased on top of v4.8.0-rc1
* fixed one improper resource deallocation on error path of dw_hdmi_bind()
* added a comment describing a mutex asked by checkpatch.pl --strict

Link to version v5: https://patchwork.kernel.org/patch/7279831/

Changes from v4 to v5:
* do I2C bus controller initialization only once in bind() as it was done
  in v1-v3 of the change.

Changes from v3 to v4, thanks to Doug and Philipp for review:
* set speed mode after software reset in dw_hdmi_i2c_init()
* by default set standard speed mode instead of fast speed mode, on iMX6Q
  this configures SCL to 100 KHz, which is compliant with HDMI 1.3a spec
* do I2C bus controller reinitialization on every data transfer,
  this change hopefully solves some observed problems on RK3288 platform
* added short functional change description to dw_hdmi.txt

v3 of the change was

  Tested-by: Philipp Zabel 

Changes from v2 to v3, thanks to Russell:
* moved register field value definitions to dw_hdmi.h
* made completions uninterruptible to avoid transfer retries if interrupted
* use one completion for both read and write transfers as in v1,
  operation_reg is removed
* redundant i2c->stat = 0 is removed from dw_hdmi_i2c_read/write()
* struct i2c_algorithm dw_hdmi_algorithm is qualified as const
* dw_hdmi_i2c_adapter() does not modify struct dw_hdmi on error path
* dw_hdmi_i2c_irq() does not modify hdmi->i2c->stat, if interrupt is
  not for I2CM
* spin lock is removed from dw_hdmi_i2c_irq()
* removed spin lock from dw_hdmi_i2c_xfer() around write to
  HDMI_IH_MUTE_I2CM_STAT0 register
* split loop over message array in dw_hdmi_i2c_xfer() to validation
  and transfer parts
* added a mutex to serialize I2C transfer requests, i2c->lock is
  completely removed
* removed if(len) check from dw_hdmi_i2c_write(), hardware supports
  only len>0 transfers
* described extension blocks <= 1 limitation in the commit message
* a number of minor clean ups

Changes from v1 to v2:
* fixed a devm_kfree() signature
* split completions for read and write operations

 .../devicetree/bindings/display/bridge/dw_hdmi.txt |   4 +-
 drivers/gpu/drm/bridge/dw-hdmi.c   | 265 -
 drivers/gpu/drm/bridge/dw-hdmi.h   |  19 ++
 3 files changed, 281 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt 
b/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
index dc1452f0d5d8..5e9a84d6e5f1 100644
--- a/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
+++ b/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
@@ -19,7 +19,9 @@ Required properties:

 Optional properties
 - reg-io-width: the width of the reg:1,4, default set to 1 if not present
-- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
+- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing,
+  if the property is omitted, a functionally reduced I2C bus
+  controller on DW HDMI is probed
 - clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec"

 Example:
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index ce3527cd0d25..cdf0a3a2e6f8 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1,14 +1,15 @@
 /*
+ * DesignWare High-Definition Multimedia Interface (HDMI) driver
+ *
+ * Copyright (C) 2013-2015 Mentor Graphics Inc.
  * Copyright (C) 2011-2013 Freescale Semiconductor, Inc.
+ * Copyright (C) 2010, Guennadi Liakhovetski 
  *
  * 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.
  *
- * Designware High-Definition Multimedia Interface (HDMI) driver
- *
- * Copyright (C) 2010, Guennadi Liakhovetski 
  */
 #include 
 #include 
@@ -101,6 +102,17 @@ struct hdmi_data_info {
struct hdmi_vmode video_mode;
 };

+struct dw_hdmi_i2c {
+   struct i2c_adapter  adap;
+
+   struct mutexlock;   /* used to serialize data transfers */
+   struct completion   cmp;
+   u8