* Re: [PATCH v2 2/2] dma-buf: Add user interfaces for dmabuf sync support
From: David Herrmann @ 2013-08-21 13:17 UTC (permalink / raw)
To: Inki Dae
Cc: dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-arm-kernel, linux-media, linaro-kernel, Maarten Lankhorst,
Sumit Semwal, kyungmin.park, myungjoo.ham
In-Reply-To: <1377081215-5948-3-git-send-email-inki.dae@samsung.com>
Hi
On Wed, Aug 21, 2013 at 12:33 PM, Inki Dae <inki.dae@samsung.com> wrote:
> This patch adds lock and poll callbacks to dma buf file operations,
> and these callbacks will be called by fcntl and select system calls.
>
> fcntl and select system calls can be used to wait for the completion
> of DMA or CPU access to a shared dmabuf. The difference of them is
> fcntl system call takes a lock after the completion but select system
> call doesn't. So in case of fcntl system call, it's useful when a task
> wants to access a shared dmabuf without any broken. On the other hand,
> it's useful when a task wants to just wait for the completion.
1)
So how is that supposed to work in user-space? I don't want to block
on a buffer, but get notified once I can lock it. So I do:
select(..dmabuf..)
Once it is finished, I want to use it:
flock(..dmabuf..)
However, how can I guarantee the flock will not block? Some other
process might have locked it in between. So I do a non-blocking
flock() and if it fails I wait again? Looks ugly and un-predictable.
2)
What do I do if some user-space program holds a lock and dead-locks?
3)
How do we do modesetting in atomic-context in the kernel? There is no
way to lock the object. But this is required for panic-handlers and
more importantly the kdb debugging hooks.
Ok, I can live with that being racy, but would still be nice to be considered.
4)
Why do we need locks? Aren't fences enough? That is, in which
situation is a lock really needed?
If we assume we have two writers A and B (DMA, CPU, GPU, whatever) and
they have no synchronization on their own. What do we win by
synchronizing their writes? Ok, yeah, we end up with either A or B and
not a mixture of both. But if we cannot predict whether we get A or B,
I don't know why we care at all? It's random, so a mixture would be
fine, too, wouldn't it?
So if user-space doesn't have any synchronization on its own, I don't
see why we need an implicit sync on a dma-buf. Could you describe a
more elaborate use-case?
I think the problems we need to fix are read/write syncs. So we have a
write that issues the DMA+write plus a fence and passes the buf plus
fence to the reader. The reader waits for the fence and then issues
the read plus fence. It passes the fence back to the writer. The
writer waits for the fence again and then issues the next write if
required.
This has the following advantages:
- fences are _guaranteed_ to finish in a given time period. Locks, on
the other hand, might never be freed (of the holder dead-locks, for
instance)
- you avoid any stalls. That is, if a writer releases a buffer and
immediately locks it again, the reader side might stall if it didn't
lock it in exactly the given window. You have no control to guarantee
the reader ever gets access. You would need a synchronization in
user-space between the writer and reader to guarantee that. This makes
the whole lock useles, doesn't it?
Cheers
David
> Changelog v2:
> - Add select system call support.
> . The purpose of this feature is to wait for the completion of DMA or
> CPU access to a dmabuf without that caller locks the dmabuf again
> after the completion.
> That is useful when caller wants to be aware of the completion of
> DMA access to the dmabuf, and the caller doesn't use intefaces for
> the DMA device driver.
>
> Signed-off-by: Inki Dae <inki.dae@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> drivers/base/dma-buf.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 81 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> index 4aca57a..f16a396 100644
> --- a/drivers/base/dma-buf.c
> +++ b/drivers/base/dma-buf.c
> @@ -29,6 +29,7 @@
> #include <linux/export.h>
> #include <linux/debugfs.h>
> #include <linux/seq_file.h>
> +#include <linux/poll.h>
> #include <linux/dmabuf-sync.h>
>
> static inline int is_dma_buf_file(struct file *);
> @@ -80,9 +81,89 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
> return dmabuf->ops->mmap(dmabuf, vma);
> }
>
> +static unsigned int dma_buf_poll(struct file *filp,
> + struct poll_table_struct *poll)
> +{
> + struct dma_buf *dmabuf;
> + struct dmabuf_sync_reservation *robj;
> + int ret = 0;
> +
> + if (!is_dma_buf_file(filp))
> + return POLLERR;
> +
> + dmabuf = filp->private_data;
> + if (!dmabuf || !dmabuf->sync)
> + return POLLERR;
> +
> + robj = dmabuf->sync;
> +
> + mutex_lock(&robj->lock);
> +
> + robj->polled = true;
> +
> + /*
> + * CPU or DMA access to this buffer has been completed, and
> + * the blocked task has been waked up. Return poll event
> + * so that the task can get out of select().
> + */
> + if (robj->poll_event) {
> + robj->poll_event = false;
> + mutex_unlock(&robj->lock);
> + return POLLIN | POLLOUT;
> + }
> +
> + /*
> + * There is no anyone accessing this buffer so just return.
> + */
> + if (!robj->locked) {
> + mutex_unlock(&robj->lock);
> + return POLLIN | POLLOUT;
> + }
> +
> + poll_wait(filp, &robj->poll_wait, poll);
> +
> + mutex_unlock(&robj->lock);
> +
> + return ret;
> +}
> +
> +static int dma_buf_lock(struct file *file, int cmd, struct file_lock *fl)
> +{
> + struct dma_buf *dmabuf;
> + unsigned int type;
> + bool wait = false;
> +
> + if (!is_dma_buf_file(file))
> + return -EINVAL;
> +
> + dmabuf = file->private_data;
> +
> + if ((fl->fl_type & F_UNLCK) = F_UNLCK) {
> + dmabuf_sync_single_unlock(dmabuf);
> + return 0;
> + }
> +
> + /* convert flock type to dmabuf sync type. */
> + if ((fl->fl_type & F_WRLCK) = F_WRLCK)
> + type = DMA_BUF_ACCESS_W;
> + else if ((fl->fl_type & F_RDLCK) = F_RDLCK)
> + type = DMA_BUF_ACCESS_R;
> + else
> + return -EINVAL;
> +
> + if (fl->fl_flags & FL_SLEEP)
> + wait = true;
> +
> + /* TODO. the locking to certain region should also be considered. */
> +
> + return dmabuf_sync_single_lock(dmabuf, type, wait);
> +}
> +
> static const struct file_operations dma_buf_fops = {
> .release = dma_buf_release,
> .mmap = dma_buf_mmap_internal,
> + .poll = dma_buf_poll,
> + .lock = dma_buf_lock,
> };
>
> /*
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] [RFC PATCH v6] dmabuf-sync: Add a buffer synchronization framework
From: Konrad Rzeszutek Wilk @ 2013-08-21 14:27 UTC (permalink / raw)
To: Inki Dae
Cc: linux-fbdev, linaro-kernel, dri-devel, kyungmin.park,
myungjoo.ham, linux-arm-kernel, linux-media
In-Reply-To: <008201ce9e4a$08d1d1a0$1a7574e0$%dae@samsung.com>
> > > +EXPORT_SYMBOL(is_dmabuf_sync_supported);
> >
> > _GPL ?
> >
> > I would also prefix it with 'dmabuf_is_sync_supported' just to make
> > all of the libraries call start with 'dmabuf'
> >
>
> Seems better. Will change it to dmabuf_is_sync_supported, and use
> EXPORT_SYMBOL_GPL.
One thing thought - while I suggest that you use GPL variant
I think you should check who the consumers are. As in, if nvidia
wants to use it it might make their lawyers unhappy - and in turn
means that their engineers won't be able to use these symbols.
So - if there is a strong argument to not have it GPL - then please
say so.
^ permalink raw reply
* Re: [RFC 00/22] OMAPDSS: DT support
From: Laurent Pinchart @ 2013-08-21 21:07 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
Nishanth Menon, Felipe Balbi, Santosh Shilimkar, Tony Lindgren
In-Reply-To: <1376037547-10859-1-git-send-email-tomi.valkeinen@ti.com>
Hi Tomi,
I'm back from holidays and have finally found time to review your patch set.
On Friday 09 August 2013 11:38:45 Tomi Valkeinen wrote:
> Hi,
>
> This is an RFC for OMAP Display DT support. The patches work fine, at least
> for me, but they are not perfect. I mostly don't have any clear questions
> about specific issues, but I would like to get feedback on the selected
> approaches in general, and also ideas how to proceed with the series.
>
> This series contains the following:
>
> DT support for the following OMAP's display subsystem devices:
> - DSS
> - DISPC
> - DPI
> - HDMI
> - VENC
> - DSI
> - (DBI/RFBI DT is not yet implemented)
>
> DT support for the following external display devices:
> - panel-dsi-cm (Generic DSI command mode panel)
> - encoder-tfp410 (DPI-to-DVI encoder)
> - connector-dvi
> - encoder-tpd12s015 (HDMI level-shifter & ESD protection)
> - hdmi-connector
> - panel-dpi (Generic DPI panel)
> - connector-analog-tv (S-Video & Composite)
>
> DT support for the following boards:
> - OMAP4 PandaBoard
> - OMAP4 SDP
> - OMAP3 BeagleBoard
> - OMAP3 Overo with Palo43 LCD expansion-board
>
> The patches are not final, and many contain quite brief descriptions.
> Binding descriptions are also still missing. The code and bindings in the
> patches should be pretty straightforward, though.
>
> The series is based on v3.11-rc2 + a couple of non-DSS fixes. The series
> can also be found from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git
> work/dss-dev-model-dt
>
> Vocabulary
> =====
>
> Display Entity - a hardware entity that takes one or more video streams as
> input and outputs one or more video streams.
>
> Upstream Entity - A display entity in the "upstream" side of the video
> stream, i.e. closer to the original video source.
>
> Downstream Entity - A display entity in the "downstream" side of the video
> stream, i.e. closer to the panel or monitor.
>
> Video pipeline - A chain of multiple display entities, starting from the
> SoC, going to the display device the user sees.
>
> Display or Panel - A display entity showing the pixels to the user
>
> Encoder - A display entity that takes video as an input and (usually)
> outputs it in some other format.
>
> Connector - HDMI/DVI/etc Connector on the board, to which an external
> monitor is connected.
>
> About Stable DT Bindings
> ============
>
> Generally speaking, the DT bindings should be stable. This brings the
> following problems:
>
> We already have DT bindings for some OMAP4 and OMAP3 boards, and OMAP4
> boards do not even have board files anymore. There are no display bindings
> for those OMAP4 boards, but the display support is currently enabled as a
> hack, by calling board-file-like code to add the display devices for the
> selected boards. So, when we add the display bindings, we should still
> support the current DT files which do not have the display bindings. Which
> would mean that we'd need to keep the hacky code forever. Considering the
> fact that the hacky code does not work quite correct in all cases, I don't
> see keeping it as a very good option.
>
> CDF (Common Display Framework) is in the works, and will most likely have
> different or more detailed bindings. Moving to CDF would mean we'd somehow
> need to still support the old OMAP bindings. In theory the display DT
> bindings should stay the same, as they represent the HW, not any SW
> constructs, but in practice I find it hard to believe the OMAP display
> bindings would be so perfect that there would be no need for changes.
>
> We most likely should somehow represent DSS clock tree in DT. That is not a
> simple task, and when we manage to do it, it again means supporting the DT
> files without clock tree data.
>
> All in all, I'm a bit scared to push the display bindings, as it's already
> clear there are changes coming. Then again, supporting the current hack for
> OMAP4 based boards is not nice at all, and has issues, so it would be really
> nice to get OMAP4 boards use proper display bindings.
Getting bindings right on the very first try is a very difficult, if not
impossible, task. We now have dedicated DT bindings reviewers and maintainers
to help here, and all DT bindings in mainline will likely go through extensive
review in the not too distant future. I don't know if there has been any
formal agreement on that, but one idea that seemed generally well approved was
to first mark new DT bindings as experimental before promoting them to stable.
On the other hand, having experimental bindings in mainline should help
stabilizing them, but that's obviously not a reason not to think them properly
from the start.
> General description of the DT bindings
> ===================
>
> All the display entities are represented as DT nodes of their own, and have
> a matching Linux driver. The display entities are organized by their control
> bus; that is, if a display entity is not controlled via any bus, it's a
> platform device, and if, say, it's controlled via i2c device, it's an i2c
> device.
I very much like that approach, and I think it's generally agreed upon.
> The exception to the above are DSI and DBI. DSI and DBI are combined control
> and video busses, but the use of the busses for control purposes is not
> independent of the video stream. Also, the the busses are, in practice,
> one-to-one links. And last, DSI and DBI display entities are often also
> controllable via, say, i2c. For these reasons there is no real Linux bus
> for DSI and DBI and thus the DSI and DBI devices are either platform
> devices or i2c devices.
That's something I'm less convinced about, but I don't have a too strong
feeling. The best implementation should get to mainline, I won't nack this
architecture just for theoretical reasons.
> The display entities are connected via "video-source" property. The video-
> source points to the upstream display entity where the video data comes
> from, and a chain of display entities thus form a full video pipeline. All
> video pipelines end with either a panel or a connector.
This is the part that bothers me the most, as I find it too limiting. In a
purely linear pipeline entities will have at most one video source, but linear
pipelines won't be enough for the future (and are actually not enough today
already). We need to support entities with more than one sink pad in the DT
bindings.
I've posted the third version of the Common Display Framework RFC
(https://lwn.net/Articles/563157/). While not formally documented, the DT
bindings are described in the cover letter. They're based on the V4L2 DT
bindings and can express arbitrary display pipelines in DT. The patch set also
provides helper functions to parse the DT bindings.
Could you please have a look at the bindings and tell me whether you can use
them for the OMAPDSS (even if you don't use the CDF helpers to start with) ?
> All the data related to a display entity, and how it is connected on the
> given board, is defined in the DT node of the display entity. This means
> that the DT node of the upstream entity does not have to be modified when
> adding support for new boards.
>
> As an example, consider OMAP3's DPI and two boards using it for panels.
> omap3.dtsi contains a node for the DPI, and the board dts files contain
> nodes for their panels. The board dts files do not change anything in the
> included DPI node. So:
>
> omap3.dtsi:
>
> dpi: encoder@0 {
> compatible = "ti,omap3-dpi";
> };
>
> omap3-board1.dts:
>
> lcd0: display@0 {
> compatible = "samsung,lte430wq-f0c", "panel-dpi";
> video-source = <&dpi>;
> data-lines = <24>;
> };
>
> omap3-board2.dts:
>
> lcd0: display@0 {
> compatible = "samsung,lte430wq-f0c", "panel-dpi";
> video-source = <&dpi>;
> data-lines = <16>;
> };
>
> The logic here is that the boards may have multiple panels that are
> connected to the same source, even if the panels can only be used one at a
> time. Each panel may thus have different properties for the bus, like the
> number of data-lines.
>
> Video bus properties
> ==========
>
> One question I've been pondering for a long time is related to the bus
> between two display entities. As an example, DPI (parallel RGB) requires
> configuring the number of datalines used. As described above, the properties
> of the video bus are represented in the downstream entity.
>
> This approach has one drawback: how to represent features specific to the
> upstream entity? Say, if OMAP's DSI has a bus-related foo-feature, which can
> be used in some scenarios, the only place where this foo-feature can be
> specified is the OMAP DSI's properties. Not in the DSI Panel's properties,
> which in the current model contains properties related to the bus.
>
> So Laurent has proposed to use V4L2-like ports, as described in
> Documentation/devicetree/bindings/media/video-interfaces.txt. I have not
> implemented such feature for OMAP DSS for the following reasons:
>
> - The current supported displays we use work fine with the current method
> - If I were to implement such system, it'd most certainly be different than
> what CDF will have.
>
> That said, the port based approach does sound good, and it would also remove
> the design issue with OMAP DPI and SDI modules as described later. So maybe
> I should just go forward with it and hope that CDF will do it in similar
> manner.
I believe we should go for the port-based approach, even if you don't use the
CDF helpers (possibly at first only).
> DSI Module ID
> ======>
> On OMAP4 we have two DSI modules. To configure the clock routing and pin
> muxing, we need to know the hardware module ID for the DSI device, i.e. is
> this Linux platform device DSI1 or DSI2. The same issue exists with other
> SoCs with multiple outputs of the same kind.
>
> With non-DT case, we used the platform device's ID directly. With DT, that
> doesn't work. I don't currently have a good solution for this, so as a
> temporary solution the DSI driver contains a table, from which it can find
> the HW module ID by using the device's base address.
You could add two output ports to the DSS, one for each DSI, and link the DSI
modules to the DSS output ports in DT. That would represent the hardware
topology, and would allow the DSI driver to know its ID based on the DSS
output port it's connected to. It's still a bit hackish in the sense that the
DSI driver will use information provided by the DSS (the output port number),
but not more than the current method.
> I believe, but I am not totally sure, that we can remove the concept of DSI
> module ID if we have a good representation of the DSS clock tree and DSI pin
> muxing in DT. The clock tree is probably a huge undertaking, but the pin
> muxing should be much easier. However, pinmuxing also is some
> complications, like the pins requiring a regulator to be enabled.
>
> Display names and aliases
> ============>
> With the board-file based model each display was given a name in the board
> file. Additionally, each display was given an alias in the style "displayX",
> where X is in incrementing number.
>
> The name could be used by the user to see which display device is what, i.e.
> on Pandaboard there are displays names "dvi" and "hdmi".
>
> The DT bindings do not have such a name. It would be simple to add a
> "nickname" property to each display node, but it just looked rather ugly so
> I have left it out.
>
> Additionally, as there's no clear order in which the displays are created,
> and thus the number in "displayX" alias could change semi-randomly, I added
> the displays to "aliases" node. This keeps the display number the same over
> boots, and also gives us some way to define a default display, i.e. which
> display to use initially if the user has not specified it.
>
> omapdss virtual device
> ===========
>
> In addition to the DSS devices matching to DSS hardware modules, we have a
> "virtual" omapdss device which does not match to any actual HW module. The
> device is there mostly for legacy reasons, but it has also allowed us to
> easily pass platform callbacks. The same device is also present in DT
> solution. It is created in code, and not present in DT bindings.
>
> Obviously, this omapdss virtual device is on the hack side, and nobody would
> mind if it would disappear.
>
> The following data is passed via omapdss device's platform data:
>
> - OMAP DSS version. In theory, the DSS revision registers should tell us
> which features the HW supports. In practice, the HW people have not
> bothered to change the revision number each time they've made changes. So
> we pass a DSS version from the platform code, based on OMAP revision
> number.
>
> - omap_dss_set_min_bus_tput() and omap_pm_get_dev_context_loss_count() to
> manage PM
>
> - DSI pin muxing functions.
>
> I have some ideas how to deduce the DSS version by poking to certain DSS
> registers, but it is not yet tested so I don't know if it will work.
That might be a stupid question, but can't you just encode the version in the
compatible string of the DSS DT node (the one currently compatible with
"ti,omap[34]-dss") ?
Version information might also need to be split/duplicated in several of the
DSS DT nodes. A quick grep through the driver source code shows that the
version is used by the submodules to infer SoC glue information. For instance
dsi_get_channel() uses the version to find the DSI module channel ID. That
information could possibly be retrieved from the links between the DSS DT
nodes.
> We could do altogether without omap_pm_get_dev_context_loss_count(), so that
> should be removable with some work. I am not sure if
> omap_dss_set_min_bus_tput() is supported via standard PM calls or not.
>
> However, the use of set_min_bus_tput() is actually a hack, as we're not
> really setting min bus tput. What we want to do is prevent OPP50. DSS clocks
> have different maximums on OPP100 and OPP50. So if DSS uses a clock over
> OPP50 limit, OPP50 cannot be entered. We prevent OPP50 by setting an
> arbitrarily high min bus tput.
>
> The DSI pin muxing should also be solvable with DT based solution, but is
> not the most trivial task and needs some work.
>
> So, I presume that at some point we can remove the omapdss device, but in
> the current solution it exists for the above reasons.
>
> DSS submodules in DT bindings
> ==============>
> The OMAP DSS modules are accessed via L4 or L3, and in that sense they
> should be on the same level in the DT bindings. However, we do not have them
> in the same level, but there is a main "dss" node, under which all the
> other DSS modules reside. The main reason for this is that the main DSS
> device needs to be enabled for the other modules to work properly, and
> having this structure makes runtime PM handle enabling DSS automatically.
>
> If I recall right, somebody (Paul?) mentioned that in the hardware there is
> actually some DSS internal bus, and thus the DT structure is good in that
> sense also.
>
> We also have nodes (and matching Linux devices) for DPI and SDI. Neither of
> them are actuall a separate IP block in the hardware, but are really more
> parts of DSS and maybe DISPC. They are still represented in the same way as
> the other DSS modules, to have similar architecture for all DSS outputs. But
> as they do not have an address-space of their own, the DT nodes use 0 and 1
> as "addresses", i.e. "dpi: encoder@0".
>
> That is rather ugly, and maybe could use some cleaning up. A V4L2 port style
> approach would probably allow us to add DPI and SDI ports as part of DSS.
I think that would really be a good idea. The DPI and SDI code could be moved
to the DSS module (it can of course still be kept in separate files as today),
and the DISPC (if I'm not mistaken) entity would just have several output
ports.
> Some of the DSS modules are actually a combination of multiple smaller
> modules. For example, the DSI module contains DSI protocol, DSI PHY and DSI
> PLL modules, each in their own address space. These could perhaps be
> presented as separate DT nodes and Linux devices, but I am not sure if that
> is a good approach or not.
What are the chances that one of those block will be upgraded and/or replaced
independently of the others in the future (I know it's a tricky question) ? It
might not be worth it going to a too fine-grained approach at the moment, but
we need to make sure that the DT bindings will allow an easy path forward if
needed.
> DT Node Names
> ======>
> I have used the following naming conventions with DT nodes:
>
> - Panels are named "display"
> - Connectors are named "connector"
> - Encoders are named "encoder"
> - DSS and DISPC are "dss" and "dispc", as they don't really match any of the
> above
>
> When appropriate, the address part of the node is the base address, as in
> "dsi1: encoder@58004000". For platform devices, I have used an increasing
> number for the address, as in "tpd12s015: encoder@1".
>
> Final words
> =====>
> So as is evident, I have things in my mind that should be improved. Maybe
> the most important question for short term future is:
>
> Can we add DSS DT bindings for OMAP4 as unstable bindings? It would give us
> some proper testing of the related code, and would also allow us to remove
> the related hacks (which don't even work quite right). However, I have no
> idea yet when the unstable DSS bindings would turn stable.
>
> If we shouldn't add the bindings as unstable, when should the bindings be
> added? Wait until CDF is in the mainline, and use that?
What about using the CDF bindings, without waiting for CDF to be in mainline ?
I believe the bindings should be upstreamed as unstable to start with anyway.
> I have not explained every piece of DSS in detail, as that would result in a
> book instead of this email, so feel free to ask for more details about any
> part.
>
> And last but not least (for me, at least =), I'm on vacation for the next
> two weeks, so answers may be delayed.
Enjoy your holidays and recharge your batteries, you will need energy when
you'll come back :-)
> Tomi Valkeinen (22):
> ARM: OMAP: remove DSS DT hack
> OMAPDSS: remove DT hacks for regulators
> ARM: OMAP2+: add omapdss_init_of()
> OMAPDSS: if dssdev->name=NULL, use alias
> OMAPDSS: get dssdev->alias from DT alias
> OMAPFB: clean up default display search
> OMAPFB: search for default display with DT alias
> OMAPDSS: Add DT support to DSS, DISPC, DPI, HDMI, VENC
> OMAPDSS: Add DT support to DSI
> ARM: omap3.dtsi: add omapdss information
> ARM: omap4.dtsi: add omapdss information
> ARM: omap4-panda.dts: add display information
> ARM: omap4-sdp.dts: add display information
> ARM: omap3-tobi.dts: add lcd (TEST)
> ARM: omap3-beagle.dts: add display information
> OMAPDSS: panel-dsi-cm: Add DT support
> OMAPDSS: encoder-tfp410: Add DT support
> OMAPDSS: connector-dvi: Add DT support
> OMAPDSS: encoder-tpd12s015: Add DT support
> OMAPDSS: hdmi-connector: Add DT support
> OMAPDSS: panel-dpi: Add DT support
> OMAPDSS: connector-analog-tv: Add DT support
>
> arch/arm/boot/dts/omap3-beagle.dts | 29 ++++++++
> arch/arm/boot/dts/omap3-tobi.dts | 33 ++++++++
> arch/arm/boot/dts/omap3.dtsi | 43 +++++++++++
> arch/arm/boot/dts/omap4-panda-common.dtsi | 48 ++++++++++++
> arch/arm/boot/dts/omap4-sdp.dts | 70 +++++++++++++++++
> arch/arm/boot/dts/omap4.dtsi | 59 +++++++++++++++
> arch/arm/mach-omap2/board-generic.c | 13 +---
> arch/arm/mach-omap2/common.h | 2 +
> arch/arm/mach-omap2/display.c | 34 +++++++++
> arch/arm/mach-omap2/dss-common.c | 23 ------
> arch/arm/mach-omap2/dss-common.h | 2 -
> .../video/omap2/displays-new/connector-analog-tv.c | 70 +++++++++++++++++
> drivers/video/omap2/displays-new/connector-dvi.c | 49 ++++++++++++
> drivers/video/omap2/displays-new/connector-hdmi.c | 36 +++++++++
> drivers/video/omap2/displays-new/encoder-tfp410.c | 54 ++++++++++++++
> .../video/omap2/displays-new/encoder-tpd12s015.c | 62 +++++++++++++++
> drivers/video/omap2/displays-new/panel-dpi.c | 75 +++++++++++++++++++
> drivers/video/omap2/displays-new/panel-dsi-cm.c | 87 +++++++++++++++++++
> drivers/video/omap2/dss/dispc.c | 7 ++
> drivers/video/omap2/dss/display.c | 23 +++++-
> drivers/video/omap2/dss/dpi.c | 8 ++
> drivers/video/omap2/dss/dsi.c | 58 +++++++++++++--
> drivers/video/omap2/dss/dss.c | 10 +++
> drivers/video/omap2/dss/hdmi.c | 11 +--
> drivers/video/omap2/dss/venc.c | 7 ++
> drivers/video/omap2/omapfb/omapfb-main.c | 67 ++++++++++++-----
> 26 files changed, 915 insertions(+), 65 deletions(-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v2] video: imxfb: Fix retrieve values from DT
From: Sascha Hauer @ 2013-08-22 7:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1376120037-23777-1-git-send-email-shc_work@mail.ru>
On Sat, Aug 10, 2013 at 11:33:57AM +0400, Alexander Shiyan wrote:
> Display settings should be retrieved from "display" node, not from
> root fb node. This patch fix this bug.
As Documentation/devicetree/bindings/video/fsl,imx-fb.txt states
fsl,dmacr and fsl,lscr1 should be read from the imxfb node, not from the
display node. This is what the current code does.
BTW it seems cmap-greyscale, cmap-inverse and cmap-static are
undocumented. I'm not really sure whether they should exist at all.
Sascha
>
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
> drivers/video/imxfb.c | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index 38733ac..8e104c4 100644
> --- a/drivers/video/imxfb.c
> +++ b/drivers/video/imxfb.c
> @@ -753,12 +753,12 @@ static int imxfb_resume(struct platform_device *dev)
> #define imxfb_resume NULL
> #endif
>
> -static int imxfb_init_fbinfo(struct platform_device *pdev)
> +static int imxfb_init_fbinfo(struct platform_device *pdev,
> + struct device_node *np)
> {
> struct imx_fb_platform_data *pdata = pdev->dev.platform_data;
> struct fb_info *info = dev_get_drvdata(&pdev->dev);
> struct imxfb_info *fbi = info->par;
> - struct device_node *np;
>
> pr_debug("%s\n",__func__);
>
> @@ -799,7 +799,6 @@ static int imxfb_init_fbinfo(struct platform_device *pdev)
> fbi->lcd_power = pdata->lcd_power;
> fbi->backlight_power = pdata->backlight_power;
> } else {
> - np = pdev->dev.of_node;
> info->var.grayscale = of_property_read_bool(np,
> "cmap-greyscale");
> fbi->cmap_inverse = of_property_read_bool(np, "cmap-inverse");
> @@ -858,6 +857,7 @@ static int imxfb_of_read_mode(struct device *dev, struct device_node *np,
>
> static int imxfb_probe(struct platform_device *pdev)
> {
> + struct device_node *display_np = NULL;
> struct imxfb_info *fbi;
> struct fb_info *info;
> struct imx_fb_platform_data *pdata;
> @@ -887,7 +887,17 @@ static int imxfb_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, info);
>
> - ret = imxfb_init_fbinfo(pdev);
> + if (pdev->dev.of_node) {
> + display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
> + if (!display_np) {
> + dev_err(&pdev->dev,
> + "No display defined in devicetree\n");
> + ret = -EINVAL;
> + goto failed_init;
> + }
> + }
> +
> + ret = imxfb_init_fbinfo(pdev, display_np);
> if (ret < 0)
> goto failed_init;
>
> @@ -898,16 +908,8 @@ static int imxfb_probe(struct platform_device *pdev)
> fbi->mode = pdata->mode;
> fbi->num_modes = pdata->num_modes;
> } else {
> - struct device_node *display_np;
> fb_mode = NULL;
>
> - display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
> - if (!display_np) {
> - dev_err(&pdev->dev, "No display defined in devicetree\n");
> - ret = -EINVAL;
> - goto failed_of_parse;
> - }
> -
> /*
> * imxfb does not support more modes, we choose only the native
> * mode.
> --
> 1.8.1.5
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* RE: [PATCH v2 2/2] dma-buf: Add user interfaces for dmabuf sync support
From: Inki Dae @ 2013-08-22 8:46 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1377081215-5948-3-git-send-email-inki.dae@samsung.com>
Thanks for your comments,
Inki Dae
> -----Original Message-----
> From: David Herrmann [mailto:dh.herrmann@gmail.com]
> Sent: Wednesday, August 21, 2013 10:17 PM
> To: Inki Dae
> Cc: dri-devel@lists.freedesktop.org; linux-fbdev@vger.kernel.org; linux-
> arm-kernel@lists.infradead.org; linux-media@vger.kernel.org; linaro-
> kernel@lists.linaro.org; Maarten Lankhorst; Sumit Semwal;
> kyungmin.park@samsung.com; myungjoo.ham@samsung.com
> Subject: Re: [PATCH v2 2/2] dma-buf: Add user interfaces for dmabuf sync
> support
>
> Hi
>
> On Wed, Aug 21, 2013 at 12:33 PM, Inki Dae <inki.dae@samsung.com> wrote:
> > This patch adds lock and poll callbacks to dma buf file operations,
> > and these callbacks will be called by fcntl and select system calls.
> >
> > fcntl and select system calls can be used to wait for the completion
> > of DMA or CPU access to a shared dmabuf. The difference of them is
> > fcntl system call takes a lock after the completion but select system
> > call doesn't. So in case of fcntl system call, it's useful when a task
> > wants to access a shared dmabuf without any broken. On the other hand,
> > it's useful when a task wants to just wait for the completion.
>
> 1)
> So how is that supposed to work in user-space? I don't want to block
> on a buffer, but get notified once I can lock it. So I do:
> select(..dmabuf..)
> Once it is finished, I want to use it:
> flock(..dmabuf..)
> However, how can I guarantee the flock will not block? Some other
> process might have locked it in between. So I do a non-blocking
> flock() and if it fails I wait again?
s/flock/fcntl
Yes, it does if you wanted to do a non-blocking fcntl. The fcntl() call will
return -EAGAIN if some other process have locked first. So user process
could retry to lock or do other work. This user process called fcntl() with
non-blocking mode so in this case, I think the user should consider two
things. One is that the fcntl() call couldn't be failed, and other is that
the call could take a lock successfully. Isn't fcntl() with a other fd also,
not dmabuf, take a same action?
>Looks ugly and un-predictable.
>
So I think this is reasonable. However, for select system call, I'm not sure
that this system call is needed yet. So I can remove it if unnecessary.
> 2)
> What do I do if some user-space program holds a lock and dead-locks?
>
I think fcntl call with a other fd also could lead same situation, and the
lock will be unlocked once the user-space program is killed because when the
process is killed, all file descriptors of the process are closed.
> 3)
> How do we do modesetting in atomic-context in the kernel? There is no
> way to lock the object. But this is required for panic-handlers and
> more importantly the kdb debugging hooks.
> Ok, I can live with that being racy, but would still be nice to be
> considered.
Yes, The lock shouldn't be called in the atomic-context. For this, will add
comments enough.
>
> 4)
> Why do we need locks? Aren't fences enough? That is, in which
> situation is a lock really needed?
> If we assume we have two writers A and B (DMA, CPU, GPU, whatever) and
> they have no synchronization on their own. What do we win by
> synchronizing their writes? Ok, yeah, we end up with either A or B and
> not a mixture of both. But if we cannot predict whether we get A or B,
> I don't know why we care at all? It's random, so a mixture would be
> fine, too, wouldn't it?
I think not so. There are some cases that the mixture wouldn't be fine. For
this, will describe it at below.
>
> So if user-space doesn't have any synchronization on its own, I don't
> see why we need an implicit sync on a dma-buf. Could you describe a
> more elaborate use-case?
Ok, first, I think I described that enough though [PATCH 0/2]. For this, you
can refer to the below link,
http://lwn.net/Articles/564208/
Anyway, there are some cases that user-space process needs the
synchronization on its own. In case of Tizen platform[1], one is between X
Client and X Server; actually, Composite Manager. Other is between Web app
based on HTML5 and Web Browser.
Please, assume that X Client draws something in a window buffer using CPU,
and then the X Client requests SWAP to X Server. And then X Server notifies
a damage event to Composite Manager. And then Composite Manager composes the
window buffer with its own back buffer using GPU. In this case, Composite
Manager calls eglSwapBuffers; internally, flushing GL commands instead of
finishing them for more performance.
As you may know, the flushing doesn't wait for the complete event from GPU
driver. And at the same time, the X Client could do other work, and also
draw something in the same buffer again. At this time, The buffer could be
broken. Because the X Client can't aware of when GPU access to the buffer is
completed without out-of-band hand shaking; the out-of-band hand shaking is
quite big overhead. That is why we need user-space locking interface, fcntl
system call.
And also there is same issue in case of Web app: Web app draws something in
a buffer using CPU, and Web browser composes the buffer with its own buffer
using GPU. At the above, you mentioned that a mixture would be fine but it's
not fine with such reasons.
[1] https://www.tizen.org/
>
> I think the problems we need to fix are read/write syncs. So we have a
> write that issues the DMA+write plus a fence and passes the buf plus
> fence to the reader. The reader waits for the fence and then issues
> the read plus fence. It passes the fence back to the writer. The
> writer waits for the fence again and then issues the next write if
> required.
>
> This has the following advantages:
> - fences are _guaranteed_ to finish in a given time period. Locks, on
> the other hand, might never be freed (of the holder dead-locks, for
> instance)
Not so. If never unlock since locked then the buffer will be unlocked by
timeout worker queue handler in a given time period.
> - you avoid any stalls. That is, if a writer releases a buffer and
> immediately locks it again, the reader side might stall if it didn't
> lock it in exactly the given window.
> You have no control to guarantee
> the reader ever gets access. You would need a synchronization in
> user-space between the writer and reader to guarantee that. This makes
> the whole lock useles, doesn't it?
>
Ah..... right. The lock mechanism cannot guarantee the write-and-then-read
order. When the writer tries to take a lock again after the reader tried to
take a lock for read but blocked, the writer don't know the fact that the
reader tried to take a lock for read so in turn, the writer would take a
lock, and the reader side would stall as you mentioned above.
I think we wouldn't need the synchronization in user-space between the write
and reader to guarantee that if we use a mechanism such as DMA fence
additionally; wound/wait mutex to avoid dead lock issue, and DMA fence to
guarantee the write-and-then-read order. For this, I will consider using the
DMA fence. Maybe it takes much time.
Thanks,
Inki Dae
> Cheers
> David
>
> > Changelog v2:
> > - Add select system call support.
> > . The purpose of this feature is to wait for the completion of DMA or
> > CPU access to a dmabuf without that caller locks the dmabuf again
> > after the completion.
> > That is useful when caller wants to be aware of the completion of
> > DMA access to the dmabuf, and the caller doesn't use intefaces for
> > the DMA device driver.
> >
> > Signed-off-by: Inki Dae <inki.dae@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> > drivers/base/dma-buf.c | 81
> ++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 81 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> > index 4aca57a..f16a396 100644
> > --- a/drivers/base/dma-buf.c
> > +++ b/drivers/base/dma-buf.c
> > @@ -29,6 +29,7 @@
> > #include <linux/export.h>
> > #include <linux/debugfs.h>
> > #include <linux/seq_file.h>
> > +#include <linux/poll.h>
> > #include <linux/dmabuf-sync.h>
> >
> > static inline int is_dma_buf_file(struct file *);
> > @@ -80,9 +81,89 @@ static int dma_buf_mmap_internal(struct file *file,
> struct vm_area_struct *vma)
> > return dmabuf->ops->mmap(dmabuf, vma);
> > }
> >
> > +static unsigned int dma_buf_poll(struct file *filp,
> > + struct poll_table_struct *poll)
> > +{
> > + struct dma_buf *dmabuf;
> > + struct dmabuf_sync_reservation *robj;
> > + int ret = 0;
> > +
> > + if (!is_dma_buf_file(filp))
> > + return POLLERR;
> > +
> > + dmabuf = filp->private_data;
> > + if (!dmabuf || !dmabuf->sync)
> > + return POLLERR;
> > +
> > + robj = dmabuf->sync;
> > +
> > + mutex_lock(&robj->lock);
> > +
> > + robj->polled = true;
> > +
> > + /*
> > + * CPU or DMA access to this buffer has been completed, and
> > + * the blocked task has been waked up. Return poll event
> > + * so that the task can get out of select().
> > + */
> > + if (robj->poll_event) {
> > + robj->poll_event = false;
> > + mutex_unlock(&robj->lock);
> > + return POLLIN | POLLOUT;
> > + }
> > +
> > + /*
> > + * There is no anyone accessing this buffer so just return.
> > + */
> > + if (!robj->locked) {
> > + mutex_unlock(&robj->lock);
> > + return POLLIN | POLLOUT;
> > + }
> > +
> > + poll_wait(filp, &robj->poll_wait, poll);
> > +
> > + mutex_unlock(&robj->lock);
> > +
> > + return ret;
> > +}
> > +
> > +static int dma_buf_lock(struct file *file, int cmd, struct file_lock
> *fl)
> > +{
> > + struct dma_buf *dmabuf;
> > + unsigned int type;
> > + bool wait = false;
> > +
> > + if (!is_dma_buf_file(file))
> > + return -EINVAL;
> > +
> > + dmabuf = file->private_data;
> > +
> > + if ((fl->fl_type & F_UNLCK) = F_UNLCK) {
> > + dmabuf_sync_single_unlock(dmabuf);
> > + return 0;
> > + }
> > +
> > + /* convert flock type to dmabuf sync type. */
> > + if ((fl->fl_type & F_WRLCK) = F_WRLCK)
> > + type = DMA_BUF_ACCESS_W;
> > + else if ((fl->fl_type & F_RDLCK) = F_RDLCK)
> > + type = DMA_BUF_ACCESS_R;
> > + else
> > + return -EINVAL;
> > +
> > + if (fl->fl_flags & FL_SLEEP)
> > + wait = true;
> > +
> > + /* TODO. the locking to certain region should also be
considered.
> */
> > +
> > + return dmabuf_sync_single_lock(dmabuf, type, wait);
> > +}
> > +
> > static const struct file_operations dma_buf_fops = {
> > .release = dma_buf_release,
> > .mmap = dma_buf_mmap_internal,
> > + .poll = dma_buf_poll,
> > + .lock = dma_buf_lock,
> > };
> >
> > /*
> > --
> > 1.7.5.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-fbdev"
> in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [RFC v2 0/1] Adding DT support to video/da8xx-fb.c
From: Darren Etheridge @ 2013-08-22 21:21 UTC (permalink / raw)
To: devicetree, tomi.valkeinen, plagnioj, linux-fbdev, detheridge; +Cc: afzal
This is part of a larger series of patches to upgrade the da8xx-fb.c driver
to support the Texas Instruments AM335x device. As part of this upgrade
we also want to add devicetree support for both the original da8xx and the
am335x. Tomi Valkeinen has reviewed the fbdev changes but he suggested
that it was prudent to extract the dt pieces and run it through the
devicetree mailing list for review.
v2: incorporates -
* review comment changes from Prabhakar Lad
* changes the .compatible naming to match that of the exiting drm
driver for am335x devices.
* Renames the devicetree bindings documentation file to match the
actual driver filename
Thanks,
Darren
Darren Etheridge (1):
video: da8xx-fb: adding dt support
.../devicetree/bindings/video/da8xx-fb.txt | 42 ++++++++++++
drivers/video/da8xx-fb.c | 66 +++++++++++++++++++-
2 files changed, 105 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/da8xx-fb.txt
^ permalink raw reply
* [PATCH] video: da8xx-fb: adding dt support
From: Darren Etheridge @ 2013-08-22 21:21 UTC (permalink / raw)
To: devicetree, tomi.valkeinen, plagnioj, linux-fbdev, detheridge; +Cc: afzal
In-Reply-To: <1377206501-30519-1-git-send-email-detheridge@ti.com>
Enhancing driver to enable probe triggered by a corresponding dt entry.
Add da8xx-fb.txt documentation to devicetree section.
Obtain fb_videomode details for the connected lcd panel using the
display timing details present in DT.
Ensure that platform data is present before checking whether platform
callback is present (the one used to control backlight). So far this
was not an issue as driver was purely non-DT triggered, but now DT
support has been added this check must be performed.
v2: squashing multiple commits from Afzal Mohammed (afzal@ti.com)
v3: remove superfluous cast
v4: expose both ti,am3352-lcdc and ti,da830-lcdc for .compatible
as driver can use enhanced features of all version of the
silicon block.
v5: addressed review comments from Prabhakar Lad
v6: Changed the .compatible naming to match the existing drm bindings
for am33xx devices
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
.../devicetree/bindings/video/da8xx-fb.txt | 42 ++++++++++++
drivers/video/da8xx-fb.c | 66 +++++++++++++++++++-
2 files changed, 105 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/da8xx-fb.txt
diff --git a/Documentation/devicetree/bindings/video/da8xx-fb.txt b/Documentation/devicetree/bindings/video/da8xx-fb.txt
new file mode 100644
index 0000000..575bb4b
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/da8xx-fb.txt
@@ -0,0 +1,42 @@
+TI LCD Controller on DA830/DA850/AM335x SoC's
+
+Required properties:
+- compatible:
+ DA830 - "ti,da8xx-tilcdc"
+ AM335x SoC's - "ti,am33xx-tilcdc"
+- reg: Address range of lcdc register set
+- interrupts: lcdc interrupt
+- display-timings: typical videomode of lcd panel, represented as child.
+ Refer Documentation/devicetree/bindings/video/display-timing.txt for
+ display timing binding details. If multiple videomodes are mentioned
+ in display timings node, typical videomode has to be mentioned as the
+ native mode or it has to be first child (driver cares only for native
+ videomode).
+
+Recommended properties:
+- ti,hwmods: Name of the hwmod associated to the LCDC
+
+Example:
+
+lcdc@4830e000 {
+ compatible = "ti,am33xx-tilcdc";
+ reg = <0x4830e000 0x1000>;
+ interrupts = <36>;
+ ti,hwmods = "lcdc";
+ status = "okay";
+ display-timings {
+ 800x480p62 {
+ clock-frequency = <30000000>;
+ hactive = <800>;
+ vactive = <480>;
+ hfront-porch = <39>;
+ hback-porch = <39>;
+ hsync-len = <47>;
+ vback-porch = <29>;
+ vfront-porch = <13>;
+ vsync-len = <2>;
+ hsync-active = <1>;
+ vsync-active = <1>;
+ };
+ };
+};
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index ea9771f..1d9aff1 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -36,6 +36,7 @@
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/lcm.h>
+#include <video/of_display_timing.h>
#include <video/da8xx-fb.h>
#include <asm/div64.h>
@@ -1297,12 +1298,54 @@ static struct fb_ops da8xx_fb_ops = {
.fb_blank = cfb_blank,
};
+static struct lcd_ctrl_config *da8xx_fb_create_cfg(struct platform_device *dev)
+{
+ struct lcd_ctrl_config *cfg;
+
+ cfg = devm_kzalloc(&dev->dev, sizeof(struct fb_videomode), GFP_KERNEL);
+ if (!cfg)
+ return NULL;
+
+ /* default values */
+
+ if (lcd_revision = LCD_VERSION_1)
+ cfg->bpp = 16;
+ else
+ cfg->bpp = 32;
+
+ /*
+ * For panels so far used with this LCDC, below statement is sufficient.
+ * For new panels, if required, struct lcd_ctrl_cfg fields to be updated
+ * with additional/modified values. Those values would have to be then
+ * obtained from dt(requiring new dt bindings).
+ */
+
+ cfg->panel_shade = COLOR_ACTIVE;
+
+ return cfg;
+}
+
static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
{
struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
struct fb_videomode *lcdc_info;
+ struct device_node *np = dev->dev.of_node;
int i;
+ if (np) {
+ lcdc_info = devm_kzalloc(&dev->dev,
+ sizeof(struct fb_videomode),
+ GFP_KERNEL);
+ if (!lcdc_info)
+ return NULL;
+
+ if (of_get_fb_videomode(np, lcdc_info, OF_USE_NATIVE_MODE)) {
+ dev_err(&dev->dev, "timings not available in DT\n");
+ return NULL;
+ }
+ return lcdc_info;
+ }
+
for (i = 0, lcdc_info = known_lcd_panels;
i < ARRAY_SIZE(known_lcd_panels); i++, lcdc_info++) {
if (strcmp(fb_pdata->type, lcdc_info->name) = 0)
@@ -1331,7 +1374,7 @@ static int fb_probe(struct platform_device *device)
int ret;
unsigned long ulcm;
- if (fb_pdata = NULL) {
+ if (fb_pdata = NULL && !device->dev.of_node) {
dev_err(&device->dev, "Can not get platform data\n");
return -ENOENT;
}
@@ -1371,7 +1414,10 @@ static int fb_probe(struct platform_device *device)
break;
}
- lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
+ if (device->dev.of_node)
+ lcd_cfg = da8xx_fb_create_cfg(device);
+ else
+ lcd_cfg = fb_pdata->controller_data;
if (!lcd_cfg) {
ret = -EINVAL;
@@ -1390,7 +1436,7 @@ static int fb_probe(struct platform_device *device)
par->dev = &device->dev;
par->lcdc_clk = tmp_lcdc_clk;
par->lcdc_clk_rate = clk_get_rate(par->lcdc_clk);
- if (fb_pdata->panel_power_ctrl) {
+ if (fb_pdata && fb_pdata->panel_power_ctrl) {
par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
par->panel_power_ctrl(1);
}
@@ -1638,6 +1684,19 @@ static int fb_resume(struct platform_device *dev)
#define fb_resume NULL
#endif
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id da8xx_fb_of_match[] = {
+ /*
+ * this driver supports version 1 and version 2 of the
+ * Texas Instruments lcd controller (lcdc) hardware block
+ */
+ {.compatible = "ti,da8xx-tilcdc", },
+ {.compatible = "ti,am33xx-tilcdc", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, da8xx_fb_of_match);
+#endif
+
static struct platform_driver da8xx_fb_driver = {
.probe = fb_probe,
.remove = fb_remove,
@@ -1646,6 +1705,7 @@ static struct platform_driver da8xx_fb_driver = {
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(da8xx_fb_of_match),
},
};
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] video: da8xx-fb: adding dt support
From: Prabhakar Lad @ 2013-08-23 7:44 UTC (permalink / raw)
To: Darren Etheridge; +Cc: devicetree, Tomi Valkeinen, plagnioj, LFBDEV, afzal
In-Reply-To: <1377206501-30519-2-git-send-email-detheridge@ti.com>
Hi Darren,
Thanks for the patch.
On Fri, Aug 23, 2013 at 2:51 AM, Darren Etheridge <detheridge@ti.com> wrote:
> Enhancing driver to enable probe triggered by a corresponding dt entry.
>
> Add da8xx-fb.txt documentation to devicetree section.
>
> Obtain fb_videomode details for the connected lcd panel using the
> display timing details present in DT.
>
> Ensure that platform data is present before checking whether platform
> callback is present (the one used to control backlight). So far this
> was not an issue as driver was purely non-DT triggered, but now DT
> support has been added this check must be performed.
>
> v2: squashing multiple commits from Afzal Mohammed (afzal@ti.com)
> v3: remove superfluous cast
> v4: expose both ti,am3352-lcdc and ti,da830-lcdc for .compatible
> as driver can use enhanced features of all version of the
> silicon block.
> v5: addressed review comments from Prabhakar Lad
> v6: Changed the .compatible naming to match the existing drm bindings
> for am33xx devices
>
> Signed-off-by: Darren Etheridge <detheridge@ti.com>
> ---
> .../devicetree/bindings/video/da8xx-fb.txt | 42 ++++++++++++
> drivers/video/da8xx-fb.c | 66 +++++++++++++++++++-
> 2 files changed, 105 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/video/da8xx-fb.txt
>
> diff --git a/Documentation/devicetree/bindings/video/da8xx-fb.txt b/Documentation/devicetree/bindings/video/da8xx-fb.txt
> new file mode 100644
> index 0000000..575bb4b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/da8xx-fb.txt
> @@ -0,0 +1,42 @@
> +TI LCD Controller on DA830/DA850/AM335x SoC's
> +
> +Required properties:
> +- compatible:
> + DA830 - "ti,da8xx-tilcdc"
> + AM335x SoC's - "ti,am33xx-tilcdc"
what about DA850 ?
Rest of the patch looks good, with that change you can add my,
Acked-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
Regards,
--Prabhakar Lad
^ permalink raw reply
* [PATCH v3 0/1] Adding DT support to video/da8xx-fb.c
From: Darren Etheridge @ 2013-08-23 18:17 UTC (permalink / raw)
To: prabhakar.csengg, devicetree, tomi.valkeinen, plagnioj,
linux-fbdev, detheridge
Cc: afzal
This is part of a larger series of patches to upgrade the da8xx-fb.c driver
to support the Texas Instruments AM335x device. As part of this upgrade
we also want to add devicetree support for both the original da8xx and the
am335x. Tomi Valkeinen has reviewed the fbdev changes but he suggested
that it was prudent to extract the dt pieces and run it through the
devicetree mailing list for review.
v2: incorporates -
* review comment changes from Prabhakar Lad
* changes the .compatible naming to match that of the exiting drm
driver for am335x devices.
* Renames the devicetree bindings documentation file to match the
actual driver filename
v3: updated documentation to make clear which compatible to use for DA850
SoC devices.
Thanks,
Darren
Darren Etheridge (1):
video: da8xx-fb: adding dt support
.../devicetree/bindings/video/da8xx-fb.txt | 42 ++++++++++++
drivers/video/da8xx-fb.c | 66 +++++++++++++++++++-
2 files changed, 105 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/da8xx-fb.txt
^ permalink raw reply
* [PATCH v3 1/1] video: da8xx-fb: adding dt support
From: Darren Etheridge @ 2013-08-23 18:17 UTC (permalink / raw)
To: prabhakar.csengg, devicetree, tomi.valkeinen, plagnioj,
linux-fbdev, detheridge
Cc: afzal
In-Reply-To: <1377281822-29659-1-git-send-email-detheridge@ti.com>
Enhancing driver to enable probe triggered by a corresponding dt entry.
Add da8xx-fb.txt documentation to devicetree section.
Obtain fb_videomode details for the connected lcd panel using the
display timing details present in DT.
Ensure that platform data is present before checking whether platform
callback is present (the one used to control backlight). So far this
was not an issue as driver was purely non-DT triggered, but now DT
support has been added this check must be performed.
v2: squashing multiple commits from Afzal Mohammed (afzal@ti.com)
v3: remove superfluous cast
v4: expose both ti,am3352-lcdc and ti,da830-lcdc for .compatible
as driver can use enhanced features of all version of the
silicon block.
v5: addressed review comments from Prabhakar Lad
v6: Changed the .compatible naming to match the existing drm bindings
for am33xx devices
v7: clarify which compatible to use in the documentation for DA850
Acked-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
.../devicetree/bindings/video/da8xx-fb.txt | 42 ++++++++++++
drivers/video/da8xx-fb.c | 66 +++++++++++++++++++-
2 files changed, 105 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/da8xx-fb.txt
diff --git a/Documentation/devicetree/bindings/video/da8xx-fb.txt b/Documentation/devicetree/bindings/video/da8xx-fb.txt
new file mode 100644
index 0000000..d86afe7
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/da8xx-fb.txt
@@ -0,0 +1,42 @@
+TI LCD Controller on DA830/DA850/AM335x SoC's
+
+Required properties:
+- compatible:
+ DA830, DA850 - "ti,da8xx-tilcdc"
+ AM335x SoC's - "ti,am33xx-tilcdc"
+- reg: Address range of lcdc register set
+- interrupts: lcdc interrupt
+- display-timings: typical videomode of lcd panel, represented as child.
+ Refer Documentation/devicetree/bindings/video/display-timing.txt for
+ display timing binding details. If multiple videomodes are mentioned
+ in display timings node, typical videomode has to be mentioned as the
+ native mode or it has to be first child (driver cares only for native
+ videomode).
+
+Recommended properties:
+- ti,hwmods: Name of the hwmod associated to the LCDC
+
+Example for am335x SoC's:
+
+lcdc@4830e000 {
+ compatible = "ti,am33xx-tilcdc";
+ reg = <0x4830e000 0x1000>;
+ interrupts = <36>;
+ ti,hwmods = "lcdc";
+ status = "okay";
+ display-timings {
+ 800x480p62 {
+ clock-frequency = <30000000>;
+ hactive = <800>;
+ vactive = <480>;
+ hfront-porch = <39>;
+ hback-porch = <39>;
+ hsync-len = <47>;
+ vback-porch = <29>;
+ vfront-porch = <13>;
+ vsync-len = <2>;
+ hsync-active = <1>;
+ vsync-active = <1>;
+ };
+ };
+};
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index ea9771f..1d9aff1 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -36,6 +36,7 @@
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/lcm.h>
+#include <video/of_display_timing.h>
#include <video/da8xx-fb.h>
#include <asm/div64.h>
@@ -1297,12 +1298,54 @@ static struct fb_ops da8xx_fb_ops = {
.fb_blank = cfb_blank,
};
+static struct lcd_ctrl_config *da8xx_fb_create_cfg(struct platform_device *dev)
+{
+ struct lcd_ctrl_config *cfg;
+
+ cfg = devm_kzalloc(&dev->dev, sizeof(struct fb_videomode), GFP_KERNEL);
+ if (!cfg)
+ return NULL;
+
+ /* default values */
+
+ if (lcd_revision = LCD_VERSION_1)
+ cfg->bpp = 16;
+ else
+ cfg->bpp = 32;
+
+ /*
+ * For panels so far used with this LCDC, below statement is sufficient.
+ * For new panels, if required, struct lcd_ctrl_cfg fields to be updated
+ * with additional/modified values. Those values would have to be then
+ * obtained from dt(requiring new dt bindings).
+ */
+
+ cfg->panel_shade = COLOR_ACTIVE;
+
+ return cfg;
+}
+
static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
{
struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
struct fb_videomode *lcdc_info;
+ struct device_node *np = dev->dev.of_node;
int i;
+ if (np) {
+ lcdc_info = devm_kzalloc(&dev->dev,
+ sizeof(struct fb_videomode),
+ GFP_KERNEL);
+ if (!lcdc_info)
+ return NULL;
+
+ if (of_get_fb_videomode(np, lcdc_info, OF_USE_NATIVE_MODE)) {
+ dev_err(&dev->dev, "timings not available in DT\n");
+ return NULL;
+ }
+ return lcdc_info;
+ }
+
for (i = 0, lcdc_info = known_lcd_panels;
i < ARRAY_SIZE(known_lcd_panels); i++, lcdc_info++) {
if (strcmp(fb_pdata->type, lcdc_info->name) = 0)
@@ -1331,7 +1374,7 @@ static int fb_probe(struct platform_device *device)
int ret;
unsigned long ulcm;
- if (fb_pdata = NULL) {
+ if (fb_pdata = NULL && !device->dev.of_node) {
dev_err(&device->dev, "Can not get platform data\n");
return -ENOENT;
}
@@ -1371,7 +1414,10 @@ static int fb_probe(struct platform_device *device)
break;
}
- lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
+ if (device->dev.of_node)
+ lcd_cfg = da8xx_fb_create_cfg(device);
+ else
+ lcd_cfg = fb_pdata->controller_data;
if (!lcd_cfg) {
ret = -EINVAL;
@@ -1390,7 +1436,7 @@ static int fb_probe(struct platform_device *device)
par->dev = &device->dev;
par->lcdc_clk = tmp_lcdc_clk;
par->lcdc_clk_rate = clk_get_rate(par->lcdc_clk);
- if (fb_pdata->panel_power_ctrl) {
+ if (fb_pdata && fb_pdata->panel_power_ctrl) {
par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
par->panel_power_ctrl(1);
}
@@ -1638,6 +1684,19 @@ static int fb_resume(struct platform_device *dev)
#define fb_resume NULL
#endif
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id da8xx_fb_of_match[] = {
+ /*
+ * this driver supports version 1 and version 2 of the
+ * Texas Instruments lcd controller (lcdc) hardware block
+ */
+ {.compatible = "ti,da8xx-tilcdc", },
+ {.compatible = "ti,am33xx-tilcdc", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, da8xx_fb_of_match);
+#endif
+
static struct platform_driver da8xx_fb_driver = {
.probe = fb_probe,
.remove = fb_remove,
@@ -1646,6 +1705,7 @@ static struct platform_driver da8xx_fb_driver = {
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(da8xx_fb_of_match),
},
};
--
1.7.0.4
^ permalink raw reply related
* [PATCH 0/4] video: da8xx-fb: numerous bugfixes
From: Darren Etheridge @ 2013-08-23 21:52 UTC (permalink / raw)
To: linux-fbdev
In developing a driver for an HDMI encoder to be attached to the LCD controller
on TI AM335x SoC I ran across a number of bugs in the da8xx-fb.c fbdev driver.
Two of them appear to have been in the driver for a while (02 and 04), one of
them is a deficiency where some LCD controller version 2 features have not been
utilized (03) and the last one was introduced in the last patch series that I
submitted for this driver (01).
These patches do not change the behavior of the AM335x EVM's platforms LCD
panel even though the current values set by the driver are wrong. I guess LCD
panels (at least the one used on AM335x EVM) must be fairly immune to bad
timings coming from the LCD controller. However the HDMI encoder simply will
not work without these fixes, and quite possibly other LCD panels could behave
the same way.
These patches apply on top of the patch series called:
"video/da8xx-fb fbdev driver enhance to support TI am335x SoC"
that was submitted by me.
Darren Etheridge (4):
video: da8xx-fb fixing incorrect porch mappings
video: da8xx-fb: fixing timing off by one errors
video: da8xx-fb: support lcdc v2 timing register expansion
video: da8xx-fb: fix the polarities of the hsync/vsync pulse
drivers/video/da8xx-fb.c | 35 +++++++++++++++++++++++++----------
1 files changed, 25 insertions(+), 10 deletions(-)
^ permalink raw reply
* [PATCH 1/4] video: da8xx-fb fixing incorrect porch mappings
From: Darren Etheridge @ 2013-08-23 21:52 UTC (permalink / raw)
To: linux-fbdev
The driver was mapping the wrong fbdev margins to the
front porch / back porch for both vertical and horizontal
timings.
This patch corrects it so that:
hfp = right margin
hbp = left margin
vbp = upper margin
vfp = lower margin
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 1d9aff1..0333a0b 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -784,10 +784,10 @@ static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
return ret;
/* Configure the vertical and horizontal sync properties. */
- lcd_cfg_vertical_sync(panel->lower_margin, panel->vsync_len,
- panel->upper_margin);
- lcd_cfg_horizontal_sync(panel->right_margin, panel->hsync_len,
- panel->left_margin);
+ lcd_cfg_vertical_sync(panel->upper_margin, panel->vsync_len,
+ panel->lower_margin);
+ lcd_cfg_horizontal_sync(panel->left_margin, panel->hsync_len,
+ panel->right_margin);
/* Configure for disply */
ret = lcd_cfg_display(cfg, panel);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/4] video: da8xx-fb: fixing timing off by one errors
From: Darren Etheridge @ 2013-08-23 21:52 UTC (permalink / raw)
To: linux-fbdev
The LCD controller represents some of the timing fields with a 0
in the register representing 1. This was not taken into account
when these registers were being set. Interestingly enough not
all of the LCDC controller timing registers implement this representation
so carefully went through the technical reference manual to only "fix"
the correct timings.
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 0333a0b..b131a6c 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -408,9 +408,9 @@ static void lcd_cfg_horizontal_sync(int back_porch, int pulse_width,
u32 reg;
reg = lcdc_read(LCD_RASTER_TIMING_0_REG) & 0xf;
- reg |= ((back_porch & 0xff) << 24)
- | ((front_porch & 0xff) << 16)
- | ((pulse_width & 0x3f) << 10);
+ reg |= (((back_porch-1) & 0xff) << 24)
+ | (((front_porch-1) & 0xff) << 16)
+ | (((pulse_width-1) & 0x3f) << 10);
lcdc_write(reg, LCD_RASTER_TIMING_0_REG);
}
@@ -422,7 +422,7 @@ static void lcd_cfg_vertical_sync(int back_porch, int pulse_width,
reg = lcdc_read(LCD_RASTER_TIMING_1_REG) & 0x3ff;
reg |= ((back_porch & 0xff) << 24)
| ((front_porch & 0xff) << 16)
- | ((pulse_width & 0x3f) << 10);
+ | (((pulse_width-1) & 0x3f) << 10);
lcdc_write(reg, LCD_RASTER_TIMING_1_REG);
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH 3/4] video: da8xx-fb: support lcdc v2 timing register expansion
From: Darren Etheridge @ 2013-08-23 21:52 UTC (permalink / raw)
To: linux-fbdev
TI LCD controller version 2 adds some extra bits in a register to
increase the available size to represent horizontal timings. This
patch allows the fbdev driver to utilize those extra bits.
This will become important for driving an HDMI encoder from the lcd
controller where some of the VESA/CEA modes require quite large porches.
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index b131a6c..278b7d7 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -412,6 +412,21 @@ static void lcd_cfg_horizontal_sync(int back_porch, int pulse_width,
| (((front_porch-1) & 0xff) << 16)
| (((pulse_width-1) & 0x3f) << 10);
lcdc_write(reg, LCD_RASTER_TIMING_0_REG);
+
+ /*
+ * LCDC Version 2 adds some extra bits that increase the allowable
+ * size of the horizontal timing registers.
+ * remember that the registers use 0 to represent 1 so all values
+ * that get set into register need to be decremented by 1
+ */
+ if(lcd_revision = LCD_VERSION_2) {
+ /* Mask off the bits we want to change */
+ reg = lcdc_read(LCD_RASTER_TIMING_2_REG) & ~0x780000ff;
+ reg |= ((front_porch-1) & 0x300) >> 8;
+ reg |= ((back_porch-1) & 0x300) >> 4;
+ reg |= ((pulse_width-1) & 0x3c0) << 21;
+ lcdc_write(reg, LCD_RASTER_TIMING_2_REG);
+ }
}
static void lcd_cfg_vertical_sync(int back_porch, int pulse_width,
--
1.7.0.4
^ permalink raw reply related
* [PATCH 4/4] video: da8xx-fb: fix the polarities of the hsync/vsync pulse
From: Darren Etheridge @ 2013-08-23 21:52 UTC (permalink / raw)
To: linux-fbdev
The polarities were being set to active low when fbdev was requesting active
high. This patch reverses it so that what is set into the LCD controller is
correct.
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 278b7d7..f768791 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -494,12 +494,12 @@ static int lcd_cfg_display(const struct lcd_ctrl_config *cfg,
else
reg &= ~LCD_SYNC_EDGE;
- if (panel->sync & FB_SYNC_HOR_HIGH_ACT)
+ if ((panel->sync & FB_SYNC_HOR_HIGH_ACT) = 0)
reg |= LCD_INVERT_LINE_CLOCK;
else
reg &= ~LCD_INVERT_LINE_CLOCK;
- if (panel->sync & FB_SYNC_VERT_HIGH_ACT)
+ if ((panel->sync & FB_SYNC_VERT_HIGH_ACT) = 0)
reg |= LCD_INVERT_FRAME_CLOCK;
else
reg &= ~LCD_INVERT_FRAME_CLOCK;
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH v11 0/8] PHY framework
From: Kishon Vijay Abraham I @ 2013-08-26 8:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1377063973-22044-1-git-send-email-kishon@ti.com>
Hi Greg,
On Wednesday 21 August 2013 11:16 AM, Kishon Vijay Abraham I wrote:
> Added a generic PHY framework that provides a set of APIs for the PHY drivers
> to create/destroy a PHY and APIs for the PHY users to obtain a reference to
> the PHY with or without using phandle.
>
> This framework will be of use only to devices that uses external PHY (PHY
> functionality is not embedded within the controller).
>
> The intention of creating this framework is to bring the phy drivers spread
> all over the Linux kernel to drivers/phy to increase code re-use and to
> increase code maintainability.
>
> Comments to make PHY as bus wasn't done because PHY devices can be part of
> other bus and making a same device attached to multiple bus leads to bad
> design.
>
> If the PHY driver has to send notification on connect/disconnect, the PHY
> driver should make use of the extcon framework. Using this susbsystem
> to use extcon framwork will have to be analysed.
>
> You can find this patch series @
> git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git testing
Looks like there are not further comments on this series. Can you take this in
your misc tree?
Thanks
Kishon
^ permalink raw reply
* [PATCH V7 0/3] Generic PHY driver for the Exynos SoC DP PHY
From: Jingoo Han @ 2013-08-26 8:55 UTC (permalink / raw)
To: 'Kishon Vijay Abraham I'
Cc: 'Greg Kroah-Hartman', linux-kernel, linux-samsung-soc,
linux-fbdev, 'devicetree', 'Kukjin Kim',
'Sylwester Nawrocki', 'Felipe Balbi',
'Tomasz Figa', 'Jean-Christophe PLAGNIOL-VILLARD',
Tomi Valkeinen, 'Jingoo Han'
This patch series adds a simple driver for the Samsung Exynos SoC
series DP transmitter PHY, using the generic PHY framework [1].
Previously the DP PHY used an internal DT node to control the PHY
power enable bit.
These patches was tested on Exynos5250.
This PATCH v7 follows:
* PATCH v6, sent on July, 9th 2013
* PATCH v5, sent on July, 8th 2013
* PATCH v4, sent on July, 2nd 2013
* PATCH v3, sent on July, 1st 2013
* PATCH v2, sent on June, 28th 2013
* PATCH v1, sent on June, 28th 2013
Changes from v6:
* Re-based on git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git
Changes from v5:
* Re-based on git://gitorious.org/linuxphy/linuxphy.git
Changes from v4:
* Marked original bindings as deprecated in 'exynos_dp.txt'
* Fixed typo of commit message.
* Added Tomasz Figa's Reviewed-by.
Changes from v3:
* Added OF dependancy.
* Removed redundant local variable 'void __iomem *addr'.
* Removed unnecessary dev_set_drvdata().
* Added a patch that remove non-DT support for Exynos
Display Port driver.
* Removed unnecessary 'struct exynos_dp_platdata'.
* Kept supporting the original bindings for DT compatibility.
Changes from v2:
* Removed redundant spinlock
* Removed 'struct phy' from 'struct exynos_dp_video_phy'
* Updated 'samsung-phy.txt', instead of creating
'samsung,exynos5250-dp-video-phy.txt'.
* Removed unnecessary additional specifier from 'phys'
DT property.
* Added 'phys', 'phy-names' properties to 'exynos_dp.txt' file.
* Added Felipe Balbi's Acked-by.
Changes from v1:
* Replaced exynos_dp_video_phy_xlate() with of_phy_simple_xlate(),
as Kishon Vijay Abraham I guided.
* Set the value of phy-cells as 0, because the phy_provider implements
only one PHY.
* Removed unnecessary header include.
* Added '#ifdef CONFIG_OF' and of_match_ptr macro.
This series depends on the generic PHY framework [1]. These patches
refer to Sylwester Nawrocki's patches about Exynos MIPI [2].
[1] http://lwn.net/Articles/564188/
[2] http://www.spinics.net/lists/linux-samsung-soc/msg20098.html
Jingoo Han (3):
phy: Add driver for Exynos DP PHY
video: exynos_dp: remove non-DT support for Exynos Display Port
video: exynos_dp: Use the generic PHY driver
.../devicetree/bindings/phy/samsung-phy.txt | 7 ++
.../devicetree/bindings/video/exynos_dp.txt | 17 +--
drivers/phy/Kconfig | 7 ++
drivers/phy/Makefile | 7 +-
drivers/phy/phy-exynos-dp-video.c | 111 ++++++++++++++++
drivers/video/exynos/Kconfig | 2 +-
drivers/video/exynos/exynos_dp_core.c | 132 ++++++--------------
drivers/video/exynos/exynos_dp_core.h | 110 ++++++++++++++++
drivers/video/exynos/exynos_dp_reg.c | 2 -
include/video/exynos_dp.h | 131 -------------------
10 files changed, 286 insertions(+), 240 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/samsung-phy.txt
create mode 100644 drivers/phy/phy-exynos-dp-video.c
delete mode 100644 include/video/exynos_dp.h
--
1.7.10.4
^ permalink raw reply
* [PATCH 1/3] phy: Add driver for Exynos DP PHY
From: Jingoo Han @ 2013-08-26 8:56 UTC (permalink / raw)
To: 'Kishon Vijay Abraham I'
Cc: 'Greg Kroah-Hartman', linux-kernel, linux-samsung-soc,
linux-fbdev, 'devicetree', 'Kukjin Kim',
'Sylwester Nawrocki', 'Felipe Balbi',
'Tomasz Figa', 'Jean-Christophe PLAGNIOL-VILLARD',
'Tomi Valkeinen', 'Jingoo Han'
In-Reply-To: <003e01cea23a$03aac850$0b0058f0$%han@samsung.com>
Add a PHY provider driver for the Samsung Exynos SoC Display Port PHY.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
.../devicetree/bindings/phy/samsung-phy.txt | 7 ++
drivers/phy/Kconfig | 7 ++
drivers/phy/Makefile | 7 +-
drivers/phy/phy-exynos-dp-video.c | 111 ++++++++++++++++++++
4 files changed, 129 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/samsung-phy.txt
create mode 100644 drivers/phy/phy-exynos-dp-video.c
diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
new file mode 100644
index 0000000..ee262238
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -0,0 +1,7 @@
+Samsung EXYNOS SoC series Display Port PHY
+-------------------------------------------------
+
+Required properties:
+- compatible : should be "samsung,exynos5250-dp-video-phy";
+- reg : offset and length of the Display Port PHY register set;
+- #phy-cells : from the generic PHY bindings, must be 0;
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index ac239ac..fed26a0 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -38,4 +38,11 @@ config TWL4030_USB
This transceiver supports high and full speed devices plus,
in host mode, low speed.
+config PHY_EXYNOS_DP_VIDEO
+ tristate "EXYNOS SoC series Display Port PHY driver"
+ depends on OF
+ select GENERIC_PHY
+ help
+ Support for Display Port PHY found on Samsung EXYNOS SoCs.
+
endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 0dd8a98..433e685 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -2,6 +2,7 @@
# Makefile for the phy drivers.
#
-obj-$(CONFIG_GENERIC_PHY) += phy-core.o
-obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o
-obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o
+obj-$(CONFIG_GENERIC_PHY) += phy-core.o
+obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o
+obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o
+obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
diff --git a/drivers/phy/phy-exynos-dp-video.c b/drivers/phy/phy-exynos-dp-video.c
new file mode 100644
index 0000000..1dbe6ce
--- /dev/null
+++ b/drivers/phy/phy-exynos-dp-video.c
@@ -0,0 +1,111 @@
+/*
+ * Samsung EXYNOS SoC series Display Port PHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Jingoo Han <jg1.han@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+/* DPTX_PHY_CONTROL register */
+#define EXYNOS_DPTX_PHY_ENABLE (1 << 0)
+
+struct exynos_dp_video_phy {
+ void __iomem *regs;
+};
+
+static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
+{
+ u32 reg;
+
+ reg = readl(state->regs);
+ if (on)
+ reg |= EXYNOS_DPTX_PHY_ENABLE;
+ else
+ reg &= ~EXYNOS_DPTX_PHY_ENABLE;
+ writel(reg, state->regs);
+
+ return 0;
+}
+
+static int exynos_dp_video_phy_power_on(struct phy *phy)
+{
+ struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+ return __set_phy_state(state, 1);
+}
+
+static int exynos_dp_video_phy_power_off(struct phy *phy)
+{
+ struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+ return __set_phy_state(state, 0);
+}
+
+static struct phy_ops exynos_dp_video_phy_ops = {
+ .power_on = exynos_dp_video_phy_power_on,
+ .power_off = exynos_dp_video_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int exynos_dp_video_phy_probe(struct platform_device *pdev)
+{
+ struct exynos_dp_video_phy *state;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ struct phy_provider *phy_provider;
+ struct phy *phy;
+
+ state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ state->regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(state->regs))
+ return PTR_ERR(state->regs);
+
+ phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
+ phy = devm_phy_create(dev, &exynos_dp_video_phy_ops, NULL);
+ if (IS_ERR(phy)) {
+ dev_err(dev, "failed to create Display Port PHY\n");
+ return PTR_ERR(phy);
+ }
+ phy_set_drvdata(phy, state);
+
+ return 0;
+}
+
+static const struct of_device_id exynos_dp_video_phy_of_match[] = {
+ { .compatible = "samsung,exynos5250-dp-video-phy" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, exynos_dp_video_phy_of_match);
+
+static struct platform_driver exynos_dp_video_phy_driver = {
+ .probe = exynos_dp_video_phy_probe,
+ .driver = {
+ .name = "exynos-dp-video-phy",
+ .owner = THIS_MODULE,
+ .of_match_table = exynos_dp_video_phy_of_match,
+ }
+};
+module_platform_driver(exynos_dp_video_phy_driver);
+
+MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
+MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
+MODULE_LICENSE("GPL v2");
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/3] video: exynos_dp: remove non-DT support for Exynos Display Port
From: Jingoo Han @ 2013-08-26 8:57 UTC (permalink / raw)
To: 'Kishon Vijay Abraham I'
Cc: 'Greg Kroah-Hartman', linux-kernel, linux-samsung-soc,
linux-fbdev, 'devicetree', 'Kukjin Kim',
'Sylwester Nawrocki', 'Felipe Balbi',
'Tomasz Figa', 'Jean-Christophe PLAGNIOL-VILLARD',
'Tomi Valkeinen', 'Jingoo Han'
In-Reply-To: <003e01cea23a$03aac850$0b0058f0$%han@samsung.com>
Exynos Display Port can be used only for Exynos SoCs. In addition,
non-DT for EXYNOS SoCs is not supported from v3.11; thus, there is
no need to support non-DT for Exynos Display Port.
The 'include/video/exynos_dp.h' file has been used for non-DT
support and the content of file include/video/exynos_dp.h is moved
to drivers/video/exynos/exynos_dp_core.h. Thus, the 'exynos_dp.h'
file is removed. Also, 'struct exynos_dp_platdata' is removed,
because it is not used any more.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
---
drivers/video/exynos/Kconfig | 2 +-
drivers/video/exynos/exynos_dp_core.c | 116 +++++++----------------------
drivers/video/exynos/exynos_dp_core.h | 109 +++++++++++++++++++++++++++
drivers/video/exynos/exynos_dp_reg.c | 2 -
include/video/exynos_dp.h | 131 ---------------------------------
5 files changed, 135 insertions(+), 225 deletions(-)
delete mode 100644 include/video/exynos_dp.h
diff --git a/drivers/video/exynos/Kconfig b/drivers/video/exynos/Kconfig
index 1b035b2..fab9019 100644
--- a/drivers/video/exynos/Kconfig
+++ b/drivers/video/exynos/Kconfig
@@ -29,7 +29,7 @@ config EXYNOS_LCD_S6E8AX0
config EXYNOS_DP
bool "EXYNOS DP driver support"
- depends on ARCH_EXYNOS
+ depends on OF && ARCH_EXYNOS
default n
help
This enables support for DP device.
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 12bbede..05fed7d 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -20,8 +20,6 @@
#include <linux/delay.h>
#include <linux/of.h>
-#include <video/exynos_dp.h>
-
#include "exynos_dp_core.h"
static int exynos_dp_init_dp(struct exynos_dp_device *dp)
@@ -894,26 +892,17 @@ static void exynos_dp_hotplug(struct work_struct *work)
dev_err(dp->dev, "unable to config video\n");
}
-#ifdef CONFIG_OF
-static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
+static struct video_info *exynos_dp_dt_parse_pdata(struct device *dev)
{
struct device_node *dp_node = dev->of_node;
- struct exynos_dp_platdata *pd;
struct video_info *dp_video_config;
- pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
- if (!pd) {
- dev_err(dev, "memory allocation for pdata failed\n");
- return ERR_PTR(-ENOMEM);
- }
dp_video_config = devm_kzalloc(dev,
sizeof(*dp_video_config), GFP_KERNEL);
-
if (!dp_video_config) {
dev_err(dev, "memory allocation for video config failed\n");
return ERR_PTR(-ENOMEM);
}
- pd->video_info = dp_video_config;
dp_video_config->h_sync_polarity of_property_read_bool(dp_node, "hsync-active-high");
@@ -960,7 +949,7 @@ static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
return ERR_PTR(-EINVAL);
}
- return pd;
+ return dp_video_config;
}
static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
@@ -1003,48 +992,30 @@ err:
static void exynos_dp_phy_init(struct exynos_dp_device *dp)
{
- u32 reg;
+ if (dp->phy_addr) {
+ u32 reg;
- reg = __raw_readl(dp->phy_addr);
- reg |= dp->enable_mask;
- __raw_writel(reg, dp->phy_addr);
+ reg = __raw_readl(dp->phy_addr);
+ reg |= dp->enable_mask;
+ __raw_writel(reg, dp->phy_addr);
+ }
}
static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
{
- u32 reg;
-
- reg = __raw_readl(dp->phy_addr);
- reg &= ~(dp->enable_mask);
- __raw_writel(reg, dp->phy_addr);
-}
-#else
-static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
-{
- return NULL;
-}
-
-static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
-{
- return -EINVAL;
-}
-
-static void exynos_dp_phy_init(struct exynos_dp_device *dp)
-{
- return;
-}
+ if (dp->phy_addr) {
+ u32 reg;
-static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
-{
- return;
+ reg = __raw_readl(dp->phy_addr);
+ reg &= ~(dp->enable_mask);
+ __raw_writel(reg, dp->phy_addr);
+ }
}
-#endif /* CONFIG_OF */
static int exynos_dp_probe(struct platform_device *pdev)
{
struct resource *res;
struct exynos_dp_device *dp;
- struct exynos_dp_platdata *pdata;
int ret = 0;
@@ -1057,21 +1028,13 @@ static int exynos_dp_probe(struct platform_device *pdev)
dp->dev = &pdev->dev;
- if (pdev->dev.of_node) {
- pdata = exynos_dp_dt_parse_pdata(&pdev->dev);
- if (IS_ERR(pdata))
- return PTR_ERR(pdata);
+ dp->video_info = exynos_dp_dt_parse_pdata(&pdev->dev);
+ if (IS_ERR(dp->video_info))
+ return PTR_ERR(dp->video_info);
- ret = exynos_dp_dt_parse_phydata(dp);
- if (ret)
- return ret;
- } else {
- pdata = pdev->dev.platform_data;
- if (!pdata) {
- dev_err(&pdev->dev, "no platform data\n");
- return -EINVAL;
- }
- }
+ ret = exynos_dp_dt_parse_phydata(dp);
+ if (ret)
+ return ret;
dp->clock = devm_clk_get(&pdev->dev, "dp");
if (IS_ERR(dp->clock)) {
@@ -1095,15 +1058,7 @@ static int exynos_dp_probe(struct platform_device *pdev)
INIT_WORK(&dp->hotplug_work, exynos_dp_hotplug);
- dp->video_info = pdata->video_info;
-
- if (pdev->dev.of_node) {
- if (dp->phy_addr)
- exynos_dp_phy_init(dp);
- } else {
- if (pdata->phy_init)
- pdata->phy_init();
- }
+ exynos_dp_phy_init(dp);
exynos_dp_init_dp(dp);
@@ -1121,18 +1076,11 @@ static int exynos_dp_probe(struct platform_device *pdev)
static int exynos_dp_remove(struct platform_device *pdev)
{
- struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
struct exynos_dp_device *dp = platform_get_drvdata(pdev);
flush_work(&dp->hotplug_work);
- if (pdev->dev.of_node) {
- if (dp->phy_addr)
- exynos_dp_phy_exit(dp);
- } else {
- if (pdata->phy_exit)
- pdata->phy_exit();
- }
+ exynos_dp_phy_exit(dp);
clk_disable_unprepare(dp->clock);
@@ -1143,20 +1091,13 @@ static int exynos_dp_remove(struct platform_device *pdev)
#ifdef CONFIG_PM_SLEEP
static int exynos_dp_suspend(struct device *dev)
{
- struct exynos_dp_platdata *pdata = dev->platform_data;
struct exynos_dp_device *dp = dev_get_drvdata(dev);
disable_irq(dp->irq);
flush_work(&dp->hotplug_work);
- if (dev->of_node) {
- if (dp->phy_addr)
- exynos_dp_phy_exit(dp);
- } else {
- if (pdata->phy_exit)
- pdata->phy_exit();
- }
+ exynos_dp_phy_exit(dp);
clk_disable_unprepare(dp->clock);
@@ -1165,16 +1106,9 @@ static int exynos_dp_suspend(struct device *dev)
static int exynos_dp_resume(struct device *dev)
{
- struct exynos_dp_platdata *pdata = dev->platform_data;
struct exynos_dp_device *dp = dev_get_drvdata(dev);
- if (dev->of_node) {
- if (dp->phy_addr)
- exynos_dp_phy_init(dp);
- } else {
- if (pdata->phy_init)
- pdata->phy_init();
- }
+ exynos_dp_phy_init(dp);
clk_prepare_enable(dp->clock);
@@ -1203,7 +1137,7 @@ static struct platform_driver exynos_dp_driver = {
.name = "exynos-dp",
.owner = THIS_MODULE,
.pm = &exynos_dp_pm_ops,
- .of_match_table = of_match_ptr(exynos_dp_match),
+ .of_match_table = exynos_dp_match,
},
};
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 6c567bbf..56cfec8 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -13,6 +13,99 @@
#ifndef _EXYNOS_DP_CORE_H
#define _EXYNOS_DP_CORE_H
+#define DP_TIMEOUT_LOOP_COUNT 100
+#define MAX_CR_LOOP 5
+#define MAX_EQ_LOOP 5
+
+enum link_rate_type {
+ LINK_RATE_1_62GBPS = 0x06,
+ LINK_RATE_2_70GBPS = 0x0a
+};
+
+enum link_lane_count_type {
+ LANE_COUNT1 = 1,
+ LANE_COUNT2 = 2,
+ LANE_COUNT4 = 4
+};
+
+enum link_training_state {
+ START,
+ CLOCK_RECOVERY,
+ EQUALIZER_TRAINING,
+ FINISHED,
+ FAILED
+};
+
+enum voltage_swing_level {
+ VOLTAGE_LEVEL_0,
+ VOLTAGE_LEVEL_1,
+ VOLTAGE_LEVEL_2,
+ VOLTAGE_LEVEL_3,
+};
+
+enum pre_emphasis_level {
+ PRE_EMPHASIS_LEVEL_0,
+ PRE_EMPHASIS_LEVEL_1,
+ PRE_EMPHASIS_LEVEL_2,
+ PRE_EMPHASIS_LEVEL_3,
+};
+
+enum pattern_set {
+ PRBS7,
+ D10_2,
+ TRAINING_PTN1,
+ TRAINING_PTN2,
+ DP_NONE
+};
+
+enum color_space {
+ COLOR_RGB,
+ COLOR_YCBCR422,
+ COLOR_YCBCR444
+};
+
+enum color_depth {
+ COLOR_6,
+ COLOR_8,
+ COLOR_10,
+ COLOR_12
+};
+
+enum color_coefficient {
+ COLOR_YCBCR601,
+ COLOR_YCBCR709
+};
+
+enum dynamic_range {
+ VESA,
+ CEA
+};
+
+enum pll_status {
+ PLL_UNLOCKED,
+ PLL_LOCKED
+};
+
+enum clock_recovery_m_value_type {
+ CALCULATED_M,
+ REGISTER_M
+};
+
+enum video_timing_recognition_type {
+ VIDEO_TIMING_FROM_CAPTURE,
+ VIDEO_TIMING_FROM_REGISTER
+};
+
+enum analog_power_block {
+ AUX_BLOCK,
+ CH0_BLOCK,
+ CH1_BLOCK,
+ CH2_BLOCK,
+ CH3_BLOCK,
+ ANALOG_TOTAL,
+ POWER_ALL
+};
+
enum dp_irq_type {
DP_IRQ_TYPE_HP_CABLE_IN,
DP_IRQ_TYPE_HP_CABLE_OUT,
@@ -20,6 +113,22 @@ enum dp_irq_type {
DP_IRQ_TYPE_UNKNOWN,
};
+struct video_info {
+ char *name;
+
+ bool h_sync_polarity;
+ bool v_sync_polarity;
+ bool interlaced;
+
+ enum color_space color_space;
+ enum dynamic_range dynamic_range;
+ enum color_coefficient ycbcr_coeff;
+ enum color_depth color_depth;
+
+ enum link_rate_type link_rate;
+ enum link_lane_count_type lane_count;
+};
+
struct link_train {
int eq_loop;
int cr_loop[4];
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 29d9d03..b70da50 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -14,8 +14,6 @@
#include <linux/io.h>
#include <linux/delay.h>
-#include <video/exynos_dp.h>
-
#include "exynos_dp_core.h"
#include "exynos_dp_reg.h"
diff --git a/include/video/exynos_dp.h b/include/video/exynos_dp.h
deleted file mode 100644
index bd8cabd..0000000
--- a/include/video/exynos_dp.h
+++ /dev/null
@@ -1,131 +0,0 @@
-/*
- * Samsung SoC DP device support
- *
- * Copyright (C) 2012 Samsung Electronics Co., Ltd.
- * Author: Jingoo Han <jg1.han@samsung.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#ifndef _EXYNOS_DP_H
-#define _EXYNOS_DP_H
-
-#define DP_TIMEOUT_LOOP_COUNT 100
-#define MAX_CR_LOOP 5
-#define MAX_EQ_LOOP 5
-
-enum link_rate_type {
- LINK_RATE_1_62GBPS = 0x06,
- LINK_RATE_2_70GBPS = 0x0a
-};
-
-enum link_lane_count_type {
- LANE_COUNT1 = 1,
- LANE_COUNT2 = 2,
- LANE_COUNT4 = 4
-};
-
-enum link_training_state {
- START,
- CLOCK_RECOVERY,
- EQUALIZER_TRAINING,
- FINISHED,
- FAILED
-};
-
-enum voltage_swing_level {
- VOLTAGE_LEVEL_0,
- VOLTAGE_LEVEL_1,
- VOLTAGE_LEVEL_2,
- VOLTAGE_LEVEL_3,
-};
-
-enum pre_emphasis_level {
- PRE_EMPHASIS_LEVEL_0,
- PRE_EMPHASIS_LEVEL_1,
- PRE_EMPHASIS_LEVEL_2,
- PRE_EMPHASIS_LEVEL_3,
-};
-
-enum pattern_set {
- PRBS7,
- D10_2,
- TRAINING_PTN1,
- TRAINING_PTN2,
- DP_NONE
-};
-
-enum color_space {
- COLOR_RGB,
- COLOR_YCBCR422,
- COLOR_YCBCR444
-};
-
-enum color_depth {
- COLOR_6,
- COLOR_8,
- COLOR_10,
- COLOR_12
-};
-
-enum color_coefficient {
- COLOR_YCBCR601,
- COLOR_YCBCR709
-};
-
-enum dynamic_range {
- VESA,
- CEA
-};
-
-enum pll_status {
- PLL_UNLOCKED,
- PLL_LOCKED
-};
-
-enum clock_recovery_m_value_type {
- CALCULATED_M,
- REGISTER_M
-};
-
-enum video_timing_recognition_type {
- VIDEO_TIMING_FROM_CAPTURE,
- VIDEO_TIMING_FROM_REGISTER
-};
-
-enum analog_power_block {
- AUX_BLOCK,
- CH0_BLOCK,
- CH1_BLOCK,
- CH2_BLOCK,
- CH3_BLOCK,
- ANALOG_TOTAL,
- POWER_ALL
-};
-
-struct video_info {
- char *name;
-
- bool h_sync_polarity;
- bool v_sync_polarity;
- bool interlaced;
-
- enum color_space color_space;
- enum dynamic_range dynamic_range;
- enum color_coefficient ycbcr_coeff;
- enum color_depth color_depth;
-
- enum link_rate_type link_rate;
- enum link_lane_count_type lane_count;
-};
-
-struct exynos_dp_platdata {
- struct video_info *video_info;
-
- void (*phy_init)(void);
- void (*phy_exit)(void);
-};
-
-#endif /* _EXYNOS_DP_H */
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/3] video: exynos_dp: Use the generic PHY driver
From: Jingoo Han @ 2013-08-26 8:57 UTC (permalink / raw)
To: 'Kishon Vijay Abraham I'
Cc: 'Greg Kroah-Hartman', linux-kernel, linux-samsung-soc,
linux-fbdev, 'devicetree', 'Kukjin Kim',
'Sylwester Nawrocki', 'Felipe Balbi',
'Tomasz Figa', 'Jean-Christophe PLAGNIOL-VILLARD',
'Tomi Valkeinen', 'Jingoo Han'
In-Reply-To: <003e01cea23a$03aac850$0b0058f0$%han@samsung.com>
Use the generic PHY API to control the DP PHY.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
---
Documentation/devicetree/bindings/video/exynos_dp.txt | 17 +++++++++--------
drivers/video/exynos/exynos_dp_core.c | 16 ++++++++++++----
drivers/video/exynos/exynos_dp_core.h | 1 +
3 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt b/Documentation/devicetree/bindings/video/exynos_dp.txt
index 84f10c1..3289d76 100644
--- a/Documentation/devicetree/bindings/video/exynos_dp.txt
+++ b/Documentation/devicetree/bindings/video/exynos_dp.txt
@@ -6,10 +6,10 @@ We use two nodes:
-dptx-phy node(defined inside dp-controller node)
For the DP-PHY initialization, we use the dptx-phy node.
-Required properties for dptx-phy:
- -reg:
+Required properties for dptx-phy: deprecated, use phys and phy-names
+ -reg: deprecated
Base address of DP PHY register.
- -samsung,enable-mask:
+ -samsung,enable-mask: deprecated
The bit-mask used to enable/disable DP PHY.
For the Panel initialization, we read data from dp-controller node.
@@ -27,6 +27,10 @@ Required properties for dp-controller:
from common clock binding: Shall be "dp".
-interrupt-parent:
phandle to Interrupt combiner node.
+ -phys:
+ from general PHY binding: the phandle for the PHY device.
+ -phy-names:
+ from general PHY binding: Should be "dp".
-samsung,color-space:
input video data format.
COLOR_RGB = 0, COLOR_YCBCR422 = 1, COLOR_YCBCR444 = 2
@@ -68,11 +72,8 @@ SOC specific portion:
clocks = <&clock 342>;
clock-names = "dp";
- dptx-phy {
- reg = <0x10040720>;
- samsung,enable-mask = <1>;
- };
-
+ phys = <&dp_phy>;
+ phy-names = "dp";
};
Board Specific portion:
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 05fed7d..5e1a715 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -19,6 +19,7 @@
#include <linux/interrupt.h>
#include <linux/delay.h>
#include <linux/of.h>
+#include <linux/phy/phy.h>
#include "exynos_dp_core.h"
@@ -960,8 +961,11 @@ static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
dp_phy_node = of_find_node_by_name(dp_phy_node, "dptx-phy");
if (!dp_phy_node) {
- dev_err(dp->dev, "could not find dptx-phy node\n");
- return -ENODEV;
+ dp->phy = devm_phy_get(dp->dev, "dp");
+ if (IS_ERR(dp->phy))
+ return PTR_ERR(dp->phy);
+ else
+ return 0;
}
if (of_property_read_u32(dp_phy_node, "reg", &phy_base)) {
@@ -992,7 +996,9 @@ err:
static void exynos_dp_phy_init(struct exynos_dp_device *dp)
{
- if (dp->phy_addr) {
+ if (dp->phy) {
+ phy_power_on(dp->phy);
+ } else if (dp->phy_addr) {
u32 reg;
reg = __raw_readl(dp->phy_addr);
@@ -1003,7 +1009,9 @@ static void exynos_dp_phy_init(struct exynos_dp_device *dp)
static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
{
- if (dp->phy_addr) {
+ if (dp->phy) {
+ phy_power_off(dp->phy);
+ } else if (dp->phy_addr) {
u32 reg;
reg = __raw_readl(dp->phy_addr);
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 56cfec8..607e36d 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -151,6 +151,7 @@ struct exynos_dp_device {
struct video_info *video_info;
struct link_train link_train;
struct work_struct hotplug_work;
+ struct phy *phy;
};
/* exynos_dp_reg.c */
--
1.7.10.4
^ permalink raw reply related
* RE: [PATCH v3 0/5] ARM: vf610: Add DCU framebuffer driver for Vybrid VF610 platform
From: Wang Huan-B18965 @ 2013-08-26 9:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1376553714-25922-1-git-send-email-b18965@freescale.com>
SGksIEplYW4tQ2hyaXN0b3BoZSwgVG9taSwNCg0KICAgICAgIERvIHlvdSBoYXZlIGFueSBjb21t
ZW50cyBmb3IgdGhlc2UgcGF0Y2hlcz8NCg0KICAgICAgIFRoYW5rcyENCg0KDQpCZXN0IFJlZ2Fy
ZHMsDQpBbGlzb24gV2FuZw0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206
IGxpbnV4LWFybS1rZXJuZWwgW21haWx0bzpsaW51eC1hcm0ta2VybmVsLQ0KPiBib3VuY2VzQGxp
c3RzLmluZnJhZGVhZC5vcmddIE9uIEJlaGFsZiBPZiBBbGlzb24gV2FuZw0KPiBTZW50OiBUaHVy
c2RheSwgQXVndXN0IDE1LCAyMDEzIDQ6MDIgUE0NCj4gVG86IHBsYWduaW9qQGpjcm9zb2Z0LmNv
bTsgdG9taS52YWxrZWluZW5AdGkuY29tOyBzaGF3bi5ndW9AbGluYXJvLm9yZzsNCj4gRXN0ZXZh
bSBGYWJpby1SNDk0OTY7IGxpbnV4LWZiZGV2QHZnZXIua2VybmVsLm9yZzsgbGludXgtYXJtLQ0K
PiBrZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZw0KPiBDYzogSmluIFpoZW5neGlvbmctUjY0MTg4
DQo+IFN1YmplY3Q6IFtQQVRDSCB2MyAwLzVdIEFSTTogdmY2MTA6IEFkZCBEQ1UgZnJhbWVidWZm
ZXIgZHJpdmVyIGZvcg0KPiBWeWJyaWQgVkY2MTAgcGxhdGZvcm0NCj4gDQo+IFRoaXMgc2VyaWVz
IGNvbnRhaW4gRENVIGZyYW1lYnVmZmVyIGRyaXZlciBmb3IgRnJlZXNjYWxlIFZ5YnJpZCBWRjYx
MA0KPiBwbGF0Zm9ybS4NCj4gDQo+IFRoZSBEaXNwbGF5IENvbnRyb2xsZXIgVW5pdCAoRENVKSBt
b2R1bGUgaXMgYSBzeXN0ZW0gbWFzdGVyIHRoYXQNCj4gZmV0Y2hlcyBncmFwaGljcyBzdG9yZWQg
aW4gaW50ZXJuYWwgb3IgZXh0ZXJuYWwgbWVtb3J5IGFuZCBkaXNwbGF5cw0KPiB0aGVtIG9uIGEg
VEZUIExDRCBwYW5lbC4gQSB3aWRlIHJhbmdlIG9mIHBhbmVsIHNpemVzIGlzIHN1cHBvcnRlZCBh
bmQNCj4gdGhlIHRpbWluZyBvZiB0aGUgaW50ZXJmYWNlIHNpZ25hbHMgaXMgaGlnaGx5IGNvbmZp
Z3VyYWJsZS4NCj4gR3JhcGhpY3MgYXJlIHJlYWQgZGlyZWN0bHkgZnJvbSBtZW1vcnkgYW5kIHRo
ZW4gYmxlbmRlZCBpbiByZWFsLXRpbWUsDQo+IHdoaWNoIGFsbG93cyBmb3IgZHluYW1pYyBjb250
ZW50IGNyZWF0aW9uIHdpdGggbWluaW1hbCBDUFUgaW50ZXJ2ZW50aW9uLg0KPiANCj4gVGhlIGZl
YXR1cmVzOg0KPiAoMSkgRnVsbCBSR0I4ODggb3V0cHV0IHRvIFRGVCBMQ0QgcGFuZWwuDQo+ICgy
KSBGb3IgdGhlIGN1cnJlbnQgTENEIHBhbmVsLCBXUVZHQSAiNDgweDI3MiIgaXMgdGVzdGVkLg0K
PiAoMykgQmxlbmRpbmcgb2YgZWFjaCBwaXhlbCB1c2luZyB1cCB0byA0IHNvdXJjZSBsYXllcnMg
ZGVwZW5kZW50IG9uDQo+IHNpemUgb2YgcGFuZWwuDQo+ICg0KSBFYWNoIGdyYXBoaWMgbGF5ZXIg
Y2FuIGJlIHBsYWNlZCB3aXRoIG9uZSBwaXhlbCByZXNvbHV0aW9uIGluDQo+IGVpdGhlciBheGlz
Lg0KPiAoNSkgRWFjaCBncmFwaGljIGxheWVyIHN1cHBvcnQgUkdCNTY1IGFuZCBSR0I4ODggZGly
ZWN0IGNvbG9ycyB3aXRob3V0DQo+IGFscGhhIGNoYW5uZWwgYW5kIEJHUkE4ODg4IGRpcmVjdCBj
b2xvcnMgd2l0aCBhbiBhbHBoYSBjaGFubmVsLg0KPiAoNikgRWFjaCBncmFwaGljIGxheWVyIHN1
cHBvcnQgYWxwaGEgYmxlbmRpbmcgd2l0aCA4LWJpdCByZXNvbHV0aW9uLg0KPiANCj4gQ2hhbmdl
cyBpbiB2MzoNCj4gLSBDb3JyZWN0IERDVV9NT0RFX0JMRU5EX0lURVIgbWFjcm8gZGVmaW5pdGlv
bi4NCj4gLSBSZW1vdmUgaGFyZGNvZGUgcGFuZWwgZGVzY3JpcHRpb24gaW4gdGhlIGRyaXZlci4g
VXNlIHRoZSB2aWRlb21vZGUNCj4gaGVscGVycyB0byBnZXQgdGhlIHJlbGV2YW50IGRhdGEgZnJv
bSBkZXZpY2V0cmVlLg0KPiAtIENvcnJlY3QgdGhlIHdyb25nIGluZGVudGF0aW9uLg0KPiAtIFVz
ZSBkZXZfKiBmb3IgcHJpbnRpbmcgbWVzc2FnZXMgaW4gZHJpdmVycy4NCj4gLSBDaGFuZ2UgY2Fs
Y19kaXZfcmF0aW8oKSB0byBmc2xfZGN1X2NhbGNfZGl2KCksIGFuZCByZXdyaXRlIHRoaXMNCj4g
ZnVuY3Rpb24uDQo+IC0gVXNlIGRldm1fcmVxdWVzdF9pcnEoKSBpbnN0ZWFkIG9mIHJlcXVlc3Rf
aXJxKCkuDQo+IC0gRHJvcCB1c2VsZXNzIGNvZGUuDQo+IC0gSW5jcmVhc2UgdGhlIGxheWVycyBu
dW1iZXIgdG8gdGhlIG1heGltdW0gNi4NCj4gLSBVc2UgZG1hX2FsbG9jX3dyaXRlY29tYmluZSgp
IGluc3RlYWQgb2YgZG1hX2FsbG9jX2NvaGVyZW50KCkuDQo+IC0gVXNlIHJ1bnRpbWUgUE0uDQo+
IA0KPiBDaGFuZ2VzIGluIHYyOg0KPiAtIEFkZCBhIGRvY3VtZW50IGZvciBEQ1UgZnJhbWVidWZm
ZXIgZHJpdmVyIHVuZGVyDQo+IERvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9mYi8u
DQo+IA0KPiAtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tDQo+IEFsaXNvbiBXYW5nICg1KToNCj4gICAgICAgQVJNOiBkdHM6IHZm
NjEwOiBBZGQgRENVIGFuZCBUQ09OIG5vZGVzDQo+ICAgICAgIEFSTTogZHRzOiB2ZjYxMC10d3I6
IEVuYWJsZSBEQ1UgYW5kIFRDT04gZGV2aWNlcw0KPiAgICAgICBBUk06IGNsazogdmY2MTA6IEFk
ZCBEQ1UgYW5kIFRDT04gY2xvY2sgc3VwcG9ydA0KPiAgICAgICBmYjogQWRkIERDVSBmcmFtZWJ1
ZmZlciBkcml2ZXIgZm9yIFZ5YnJpZCBWRjYxMCBwbGF0Zm9ybQ0KPiAgICAgICBEb2N1bWVudGF0
aW9uOiBEVDogQWRkIERDVSBmcmFtZWJ1ZmZlciBkcml2ZXINCj4gDQo+ICBEb2N1bWVudGF0aW9u
L2RldmljZXRyZWUvYmluZGluZ3MvZmIvZnNsLWRjdS1mYi50eHQgfCAgIDY3ICsrKysrDQo+ICBh
cmNoL2FybS9ib290L2R0cy92ZjYxMC10d3IuZHRzICAgICAgICAgICAgICAgICAgICAgfCAgIDMy
ICsrKw0KPiAgYXJjaC9hcm0vYm9vdC9kdHMvdmY2MTAuZHRzaSAgICAgICAgICAgICAgICAgICAg
ICAgIHwgICAxOSArLQ0KPiAgYXJjaC9hcm0vbWFjaC1pbXgvY2xrLXZmNjEwLmMgICAgICAgICAg
ICAgICAgICAgICAgIHwgICAgNSArDQo+ICBkcml2ZXJzL3ZpZGVvL0tjb25maWcgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgfCAgIDEwICsNCj4gIGRyaXZlcnMvdmlkZW8vTWFrZWZpbGUg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICB8ICAgIDEgKw0KPiAgZHJpdmVycy92aWRlby9m
c2wtZGN1LWZiLmMgICAgICAgICAgICAgICAgICAgICAgICAgIHwgMTA5NQ0KPiArKysrKysrKysr
KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr
KysrKw0KPiArKysrKysrKysNCj4gIGluY2x1ZGUvZHQtYmluZGluZ3MvY2xvY2svdmY2MTAtY2xv
Y2suaCAgICAgICAgICAgICB8ICAgIDMgKy0NCj4gIDggZmlsZXMgY2hhbmdlZCwgMTIzMCBpbnNl
cnRpb25zKCspLCAyIGRlbGV0aW9ucygtKSAgY3JlYXRlIG1vZGUNCj4gMTAwNjQ0IERvY3VtZW50
YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9mYi9mc2wtZGN1LWZiLnR4dA0KPiAgY3JlYXRlIG1v
ZGUgMTAwNjQ0IGRyaXZlcnMvdmlkZW8vZnNsLWRjdS1mYi5jDQo+IA0KPiANCj4gDQo+IF9fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fDQo+IGxpbnV4LWFybS1r
ZXJuZWwgbWFpbGluZyBsaXN0DQo+IGxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9y
Zw0KPiBodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LWFy
bS1rZXJuZWwNCg0K
^ permalink raw reply
* Re: [PATCH V7 0/3] Generic PHY driver for the Exynos SoC DP PHY
From: Kishon Vijay Abraham I @ 2013-08-26 9:55 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Greg Kroah-Hartman', linux-kernel, linux-samsung-soc,
linux-fbdev, 'devicetree', 'Kukjin Kim',
'Sylwester Nawrocki', 'Felipe Balbi',
'Tomasz Figa', 'Jean-Christophe PLAGNIOL-VILLARD',
Tomi Valkeinen
In-Reply-To: <003e01cea23a$03aac850$0b0058f0$%han@samsung.com>
On Monday 26 August 2013 02:25 PM, Jingoo Han wrote:
> This patch series adds a simple driver for the Samsung Exynos SoC
> series DP transmitter PHY, using the generic PHY framework [1].
> Previously the DP PHY used an internal DT node to control the PHY
> power enable bit.
>
> These patches was tested on Exynos5250.
>
> This PATCH v7 follows:
> * PATCH v6, sent on July, 9th 2013
> * PATCH v5, sent on July, 8th 2013
> * PATCH v4, sent on July, 2nd 2013
> * PATCH v3, sent on July, 1st 2013
> * PATCH v2, sent on June, 28th 2013
> * PATCH v1, sent on June, 28th 2013
>
> Changes from v6:
> * Re-based on git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git
>
> Changes from v5:
> * Re-based on git://gitorious.org/linuxphy/linuxphy.git
>
> Changes from v4:
> * Marked original bindings as deprecated in 'exynos_dp.txt'
> * Fixed typo of commit message.
> * Added Tomasz Figa's Reviewed-by.
>
> Changes from v3:
> * Added OF dependancy.
> * Removed redundant local variable 'void __iomem *addr'.
> * Removed unnecessary dev_set_drvdata().
> * Added a patch that remove non-DT support for Exynos
> Display Port driver.
> * Removed unnecessary 'struct exynos_dp_platdata'.
> * Kept supporting the original bindings for DT compatibility.
>
> Changes from v2:
> * Removed redundant spinlock
> * Removed 'struct phy' from 'struct exynos_dp_video_phy'
> * Updated 'samsung-phy.txt', instead of creating
> 'samsung,exynos5250-dp-video-phy.txt'.
> * Removed unnecessary additional specifier from 'phys'
> DT property.
> * Added 'phys', 'phy-names' properties to 'exynos_dp.txt' file.
> * Added Felipe Balbi's Acked-by.
>
> Changes from v1:
> * Replaced exynos_dp_video_phy_xlate() with of_phy_simple_xlate(),
> as Kishon Vijay Abraham I guided.
> * Set the value of phy-cells as 0, because the phy_provider implements
> only one PHY.
> * Removed unnecessary header include.
> * Added '#ifdef CONFIG_OF' and of_match_ptr macro.
>
> This series depends on the generic PHY framework [1]. These patches
> refer to Sylwester Nawrocki's patches about Exynos MIPI [2].
>
> [1] http://lwn.net/Articles/564188/
> [2] http://www.spinics.net/lists/linux-samsung-soc/msg20098.html
>
> Jingoo Han (3):
> phy: Add driver for Exynos DP PHY
> video: exynos_dp: remove non-DT support for Exynos Display Port
> video: exynos_dp: Use the generic PHY driver
FWIW,
Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
^ permalink raw reply
* [patch] tgafb: potential NULL dereference in init
From: Dan Carpenter @ 2013-08-26 14:56 UTC (permalink / raw)
To: linux-fbdev
Static checkers complain that there are paths where "tga_type_name" can
be NULL. I've re-arranged the code slightly so that's impossible.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/video/tgafb.c b/drivers/video/tgafb.c
index c9c8e5a..2dcaf2e 100644
--- a/drivers/video/tgafb.c
+++ b/drivers/video/tgafb.c
@@ -1475,7 +1475,7 @@ tgafb_init_fix(struct fb_info *info)
int tga_bus_pci = TGA_BUS_PCI(par->dev);
int tga_bus_tc = TGA_BUS_TC(par->dev);
u8 tga_type = par->tga_type;
- const char *tga_type_name = NULL;
+ const char *tga_type_name;
switch (tga_type) {
case TGA_TYPE_8PLANE:
@@ -1496,10 +1496,9 @@ tgafb_init_fix(struct fb_info *info)
if (tga_bus_tc)
tga_type_name = "Digital ZLX-E3";
break;
- default:
- tga_type_name = "Unknown";
- break;
}
+ if (!tga_type_name)
+ tga_type_name = "Unknown";
strlcpy(info->fix.id, tga_type_name, sizeof(info->fix.id));
^ permalink raw reply related
* Re: [patch] tgafb: potential NULL dereference in init
From: Geert Uytterhoeven @ 2013-08-26 17:51 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20130826145610.GA12428@elgon.mountain>
On Mon, Aug 26, 2013 at 4:56 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> --- a/drivers/video/tgafb.c
> +++ b/drivers/video/tgafb.c
> @@ -1475,7 +1475,7 @@ tgafb_init_fix(struct fb_info *info)
> int tga_bus_pci = TGA_BUS_PCI(par->dev);
> int tga_bus_tc = TGA_BUS_TC(par->dev);
> u8 tga_type = par->tga_type;
> - const char *tga_type_name = NULL;
> + const char *tga_type_name;
Now the real compiler (at least some versions of gcc) will complain
about an uninitialized variable...
> switch (tga_type) {
> case TGA_TYPE_8PLANE:
> @@ -1496,10 +1496,9 @@ tgafb_init_fix(struct fb_info *info)
> if (tga_bus_tc)
> tga_type_name = "Digital ZLX-E3";
> break;
> - default:
> - tga_type_name = "Unknown";
> - break;
> }
> + if (!tga_type_name)
It will only by NULL if the garbage on the stack was NULL...
> + tga_type_name = "Unknown";
>
> strlcpy(info->fix.id, tga_type_name, sizeof(info->fix.id));
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [patch] tgafb: potential NULL dereference in init
From: Dan Carpenter @ 2013-08-27 1:16 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20130826145610.GA12428@elgon.mountain>
On Mon, Aug 26, 2013 at 07:51:04PM +0200, Geert Uytterhoeven wrote:
> On Mon, Aug 26, 2013 at 4:56 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > --- a/drivers/video/tgafb.c
> > +++ b/drivers/video/tgafb.c
> > @@ -1475,7 +1475,7 @@ tgafb_init_fix(struct fb_info *info)
> > int tga_bus_pci = TGA_BUS_PCI(par->dev);
> > int tga_bus_tc = TGA_BUS_TC(par->dev);
> > u8 tga_type = par->tga_type;
> > - const char *tga_type_name = NULL;
> > + const char *tga_type_name;
>
> Now the real compiler (at least some versions of gcc) will complain
> about an uninitialized variable...
Oh crap! The compiler is totally correct here. I don't know what I was
thinking. I've just double checked now and my compiler does not catch
this (GCC 4.7.2).
Sorry about that.
regards,
dan carpenter
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox