diff mbox

[5/5] OMAPFB: connect ovl managers to all dssdevs

Message ID 1354881309-17625-5-git-send-email-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tomi Valkeinen Dec. 7, 2012, 11:55 a.m. UTC
Commit 5d89bcc341771d95e3a2996218e5949a6627f59e (OMAPDSS: remove initial
display code from omapdss) moved setting up the initial overlay, overlay
manager, output and display connections from omapdss to omapfb.

However, currently omapfb only handles the connection related to the
default display, which means that no overlay managers are connected to
other displays.

This patch changes omapfb to go through all dssdevs, and connect an
overlay manager to them.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/omapfb/omapfb-main.c |   38 +++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 14 deletions(-)

Comments

archit taneja Dec. 10, 2012, 7:34 a.m. UTC | #1
Hi,

On Friday 07 December 2012 05:25 PM, Tomi Valkeinen wrote:
> Commit 5d89bcc341771d95e3a2996218e5949a6627f59e (OMAPDSS: remove initial
> display code from omapdss) moved setting up the initial overlay, overlay
> manager, output and display connections from omapdss to omapfb.
>
> However, currently omapfb only handles the connection related to the
> default display, which means that no overlay managers are connected to
> other displays.
>
> This patch changes omapfb to go through all dssdevs, and connect an
> overlay manager to them.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/video/omap2/omapfb/omapfb-main.c |   38 +++++++++++++++++++-----------
>   1 file changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/video/omap2/omapfb/omapfb-main.c b/drivers/video/omap2/omapfb/omapfb-main.c
> index 1df973e..24739fc 100644
> --- a/drivers/video/omap2/omapfb/omapfb-main.c
> +++ b/drivers/video/omap2/omapfb/omapfb-main.c
> @@ -2353,27 +2353,37 @@ static int omapfb_init_display(struct omapfb2_device *fbdev,
>   }
>
>   static int omapfb_init_connections(struct omapfb2_device *fbdev,
> -		struct omap_dss_device *dssdev)
> +		struct omap_dss_device *def_dssdev)
>   {
>   	int i, r;
> -	struct omap_overlay_manager *mgr = NULL;
> +	struct omap_overlay_manager *mgr;
>
> -	for (i = 0; i < fbdev->num_managers; i++) {
> -		mgr = fbdev->managers[i];
> -
> -		if (dssdev->channel == mgr->id)
> -			break;
> +	if (!def_dssdev->output) {
> +		dev_err(fbdev->dev, "no output for the default display\n");
> +		return -EINVAL;
>   	}
>
> -	if (i == fbdev->num_managers)
> -		return -ENODEV;
> +	for (i = 0; i < fbdev->num_displays; ++i) {
> +		struct omap_dss_device *dssdev = fbdev->displays[i].dssdev;
> +		struct omap_dss_output *out = dssdev->output;
>
> -	if (mgr->output)
> -		mgr->unset_output(mgr);
> +		mgr = omap_dss_get_overlay_manager(dssdev->channel);

This dssdev->channel reference is something we would want to get rid of 
eventually, right?

At the point omapfb_init_connections() is called, we would have all the 
omap_dss_devices registered, right? So at this point, omapfb will have 
an overall view of how the panels need to be connected to DSS.

I think we can try to find a manager here for dssdev rather than using 
dssdev->channel directly. The dssdev's output could connect to a few 
managers. We would want to chose managers for each dssdev output in such 
a way that all outputs have a manager. I guess there would be multiple 
combinations for this, but it would be okay to pick any one of them.

I think we would need some recursive or backtracking sort of approach to 
get a desired combination. We can figure about how to make it work 
later, but do you agree if this is a right way to get rid of 
dssdev->channel?

Also, we would need to do this for omapdrm separately using it's own 
encoder/connector entities.

Archit

>
> -	r = mgr->set_output(mgr, dssdev->output);
> -	if (r)
> -		return r;
> +		if (!mgr || !out)
> +			continue;
> +
> +		if (mgr->output)
> +			mgr->unset_output(mgr);
> +
> +		mgr->set_output(mgr, out);
> +	}
> +
> +	mgr = def_dssdev->output->manager;
> +
> +	if (!mgr) {
> +		dev_err(fbdev->dev, "no ovl manager for the default display\n");
> +		return -EINVAL;
> +	}
>
>   	for (i = 0; i < fbdev->num_overlays; i++) {
>   		struct omap_overlay *ovl = fbdev->overlays[i];
>

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomi Valkeinen Dec. 10, 2012, 8:03 a.m. UTC | #2
On 2012-12-10 09:34, Archit Taneja wrote:
> Hi,
> 
> On Friday 07 December 2012 05:25 PM, Tomi Valkeinen wrote:
>> Commit 5d89bcc341771d95e3a2996218e5949a6627f59e (OMAPDSS: remove initial
>> display code from omapdss) moved setting up the initial overlay, overlay
>> manager, output and display connections from omapdss to omapfb.
>>
>> However, currently omapfb only handles the connection related to the
>> default display, which means that no overlay managers are connected to
>> other displays.
>>
>> This patch changes omapfb to go through all dssdevs, and connect an
>> overlay manager to them.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>   drivers/video/omap2/omapfb/omapfb-main.c |   38
>> +++++++++++++++++++-----------
>>   1 file changed, 24 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/video/omap2/omapfb/omapfb-main.c
>> b/drivers/video/omap2/omapfb/omapfb-main.c
>> index 1df973e..24739fc 100644
>> --- a/drivers/video/omap2/omapfb/omapfb-main.c
>> +++ b/drivers/video/omap2/omapfb/omapfb-main.c
>> @@ -2353,27 +2353,37 @@ static int omapfb_init_display(struct
>> omapfb2_device *fbdev,
>>   }
>>
>>   static int omapfb_init_connections(struct omapfb2_device *fbdev,
>> -        struct omap_dss_device *dssdev)
>> +        struct omap_dss_device *def_dssdev)
>>   {
>>       int i, r;
>> -    struct omap_overlay_manager *mgr = NULL;
>> +    struct omap_overlay_manager *mgr;
>>
>> -    for (i = 0; i < fbdev->num_managers; i++) {
>> -        mgr = fbdev->managers[i];
>> -
>> -        if (dssdev->channel == mgr->id)
>> -            break;
>> +    if (!def_dssdev->output) {
>> +        dev_err(fbdev->dev, "no output for the default display\n");
>> +        return -EINVAL;
>>       }
>>
>> -    if (i == fbdev->num_managers)
>> -        return -ENODEV;
>> +    for (i = 0; i < fbdev->num_displays; ++i) {
>> +        struct omap_dss_device *dssdev = fbdev->displays[i].dssdev;
>> +        struct omap_dss_output *out = dssdev->output;
>>
>> -    if (mgr->output)
>> -        mgr->unset_output(mgr);
>> +        mgr = omap_dss_get_overlay_manager(dssdev->channel);
> 
> This dssdev->channel reference is something we would want to get rid of
> eventually, right?

Yes.

> At the point omapfb_init_connections() is called, we would have all the
> omap_dss_devices registered, right? So at this point, omapfb will have
> an overall view of how the panels need to be connected to DSS.
> 
> I think we can try to find a manager here for dssdev rather than using
> dssdev->channel directly. The dssdev's output could connect to a few
> managers. We would want to chose managers for each dssdev output in such
> a way that all outputs have a manager. I guess there would be multiple
> combinations for this, but it would be okay to pick any one of them.
> 
> I think we would need some recursive or backtracking sort of approach to
> get a desired combination. We can figure about how to make it work
> later, but do you agree if this is a right way to get rid of
> dssdev->channel?

Yes, I think that's a sensible approach. The thing I don't like about it
is that it requires omapfb/omapdrm to create the mgr-output connections.
They shouldn't really be interested in that. All they want is a display
device that works, and a way to get the mgr used for the display.

Well, I think we can hide the implementation inside omapdss, so that
omapfb/omapdrm will just need to call one omapdss func when they are
started, which will connect the mgrs to outputs.

A downside with setting up the mgr-output links at one go, when
omapfb/omapdrm starts, is that it creates a strict load order
restriction between omapdss, panels and omapfb/omapdrm. The drivers will
need to be loaded in that order, or things won't work.

Another option would be to pass information about mgr-output links from
the board files (or DT data) to the omapdss driver, so that omapdss
could setup them at probe time. With this option omapfb/omapdrm doesn't
need to care about this, and it doesn't create load order restriction.
But mgr-output links are something that I'd really like to handle inside
the drivers, not something that needs to be passed via platform data.

Third option, which is the best, but also something I have no idea how
to implement, would be to create the mgr-output links dynamically when
needed. The problem here is, of course, that a mgr could be allocated to
an output, only to be later found out that that particular mgr is needed
for another output.

But this is something we could study a bit: can we create such mgr
allocation system, that no matter what outputs the board uses, it'll
just work.

So, for example, on omap4, LCD2 mgr can be used for DPI, DSI2, DSI1, and
RFBI. LCD1 can be used for RFBI and DSI1. If we know that DSI1 and RFBI
cannot be used at the same time, we're free to give LCD1 to either one.
And if we know that DPI and DSI2 cannot be used at the same time, we're
also free to give LCD2 to either one. And if that's the case, there are
no conflicts.

I don't know if we can find such allocation for all current omaps, and I
know it's slightly risky as the next omap could have limitations even if
current ones do not.

> Also, we would need to do this for omapdrm separately using it's own
> encoder/connector entities.

Yep. That's also a negative side: both omapfb/omapdrm will need to
implement the same stuff, even if neither of them are really interested
in that stuff.

 Tomi
archit taneja Dec. 10, 2012, 9:54 a.m. UTC | #3
On Monday 10 December 2012 01:33 PM, Tomi Valkeinen wrote:
> On 2012-12-10 09:34, Archit Taneja wrote:
>> Hi,
>>
>> On Friday 07 December 2012 05:25 PM, Tomi Valkeinen wrote:
>>> Commit 5d89bcc341771d95e3a2996218e5949a6627f59e (OMAPDSS: remove initial
>>> display code from omapdss) moved setting up the initial overlay, overlay
>>> manager, output and display connections from omapdss to omapfb.
>>>
>>> However, currently omapfb only handles the connection related to the
>>> default display, which means that no overlay managers are connected to
>>> other displays.
>>>
>>> This patch changes omapfb to go through all dssdevs, and connect an
>>> overlay manager to them.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> ---
>>>    drivers/video/omap2/omapfb/omapfb-main.c |   38
>>> +++++++++++++++++++-----------
>>>    1 file changed, 24 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/video/omap2/omapfb/omapfb-main.c
>>> b/drivers/video/omap2/omapfb/omapfb-main.c
>>> index 1df973e..24739fc 100644
>>> --- a/drivers/video/omap2/omapfb/omapfb-main.c
>>> +++ b/drivers/video/omap2/omapfb/omapfb-main.c
>>> @@ -2353,27 +2353,37 @@ static int omapfb_init_display(struct
>>> omapfb2_device *fbdev,
>>>    }
>>>
>>>    static int omapfb_init_connections(struct omapfb2_device *fbdev,
>>> -        struct omap_dss_device *dssdev)
>>> +        struct omap_dss_device *def_dssdev)
>>>    {
>>>        int i, r;
>>> -    struct omap_overlay_manager *mgr = NULL;
>>> +    struct omap_overlay_manager *mgr;
>>>
>>> -    for (i = 0; i < fbdev->num_managers; i++) {
>>> -        mgr = fbdev->managers[i];
>>> -
>>> -        if (dssdev->channel == mgr->id)
>>> -            break;
>>> +    if (!def_dssdev->output) {
>>> +        dev_err(fbdev->dev, "no output for the default display\n");
>>> +        return -EINVAL;
>>>        }
>>>
>>> -    if (i == fbdev->num_managers)
>>> -        return -ENODEV;
>>> +    for (i = 0; i < fbdev->num_displays; ++i) {
>>> +        struct omap_dss_device *dssdev = fbdev->displays[i].dssdev;
>>> +        struct omap_dss_output *out = dssdev->output;
>>>
>>> -    if (mgr->output)
>>> -        mgr->unset_output(mgr);
>>> +        mgr = omap_dss_get_overlay_manager(dssdev->channel);
>>
>> This dssdev->channel reference is something we would want to get rid of
>> eventually, right?
>
> Yes.
>
>> At the point omapfb_init_connections() is called, we would have all the
>> omap_dss_devices registered, right? So at this point, omapfb will have
>> an overall view of how the panels need to be connected to DSS.
>>
>> I think we can try to find a manager here for dssdev rather than using
>> dssdev->channel directly. The dssdev's output could connect to a few
>> managers. We would want to chose managers for each dssdev output in such
>> a way that all outputs have a manager. I guess there would be multiple
>> combinations for this, but it would be okay to pick any one of them.
>>
>> I think we would need some recursive or backtracking sort of approach to
>> get a desired combination. We can figure about how to make it work
>> later, but do you agree if this is a right way to get rid of
>> dssdev->channel?
>
> Yes, I think that's a sensible approach. The thing I don't like about it
> is that it requires omapfb/omapdrm to create the mgr-output connections.
> They shouldn't really be interested in that. All they want is a display
> device that works, and a way to get the mgr used for the display.
>
> Well, I think we can hide the implementation inside omapdss, so that
> omapfb/omapdrm will just need to call one omapdss func when they are
> started, which will connect the mgrs to outputs.
>
> A downside with setting up the mgr-output links at one go, when
> omapfb/omapdrm starts, is that it creates a strict load order
> restriction between omapdss, panels and omapfb/omapdrm. The drivers will
> need to be loaded in that order, or things won't work.

Yes, that's true.

>
> Another option would be to pass information about mgr-output links from
> the board files (or DT data) to the omapdss driver, so that omapdss
> could setup them at probe time. With this option omapfb/omapdrm doesn't
> need to care about this, and it doesn't create load order restriction.
> But mgr-output links are something that I'd really like to handle inside
> the drivers, not something that needs to be passed via platform data.

This would definitely make things simpler, but if this parameter is put 
in a panel's DT, it would become omap specific. We could add this info 
to the DT corresponding to omapdss.

>
> Third option, which is the best, but also something I have no idea how
> to implement, would be to create the mgr-output links dynamically when
> needed. The problem here is, of course, that a mgr could be allocated to
> an output, only to be later found out that that particular mgr is needed
> for another output.
>
> But this is something we could study a bit: can we create such mgr
> allocation system, that no matter what outputs the board uses, it'll
> just work.

Yes, that would be quite useful. But I think we'll hit situations where 
it is sort of impossible to prevent the above situation.

When an output needs a manager. We could study the current state of the 
system by splitting managers into 2 sets:

A: managers which already have outputs connected to them
B: managers which don't have an output, but might get connected to one 
in the future.

managers in A are lost, and we can't detach them, we would need to at 
least disable/reenable the panel with a new manager connected to the output.

we need to find one from B such that maximum outputs(or some other 
weightage factor) will still be supported after this new link is made.

The system will initially have all managers in B, but eventually 
managers will move to A. We need to move one manager from B to A for 
every mgr-output link.

I guess I just described the problem in a more mathematical way, without 
providing any solution :), but it does look like an optimisation problem.

>
> So, for example, on omap4, LCD2 mgr can be used for DPI, DSI2, DSI1, and
> RFBI. LCD1 can be used for RFBI and DSI1. If we know that DSI1 and RFBI
> cannot be used at the same time, we're free to give LCD1 to either one.

But they can be used together, can't they? LCD1->DSI1 and LCD2->RFBI. 
Are you creating this constraint by assuming what the board is like? Or 
is this a constraint of OMAP DSS HW?


> And if we know that DPI and DSI2 cannot be used at the same time, we're
> also free to give LCD2 to either one. And if that's the case, there are
> no conflicts.

This is also possible at the same time: TV->DPI and LCD2->DSI2

>
> I don't know if we can find such allocation for all current omaps, and I
> know it's slightly risky as the next omap could have limitations even if
> current ones do not.

I don't understand the example so well, but I get your point of taking 
advantage of such limitations.

>
>> Also, we would need to do this for omapdrm separately using it's own
>> encoder/connector entities.
>
> Yep. That's also a negative side: both omapfb/omapdrm will need to
> implement the same stuff, even if neither of them are really interested
> in that stuff.

Yes. I wonder if crtcs, encoders and connectors already have some sort 
of helpers for this?

Archit

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomi Valkeinen Dec. 10, 2012, 10:07 a.m. UTC | #4
On 2012-12-10 11:54, Archit Taneja wrote:
> On Monday 10 December 2012 01:33 PM, Tomi Valkeinen wrote:

>> Another option would be to pass information about mgr-output links from
>> the board files (or DT data) to the omapdss driver, so that omapdss
>> could setup them at probe time. With this option omapfb/omapdrm doesn't
>> need to care about this, and it doesn't create load order restriction.
>> But mgr-output links are something that I'd really like to handle inside
>> the drivers, not something that needs to be passed via platform data.
> 
> This would definitely make things simpler, but if this parameter is put
> in a panel's DT, it would become omap specific. We could add this info
> to the DT corresponding to omapdss.

Yes, I meant it should be omapdss platform data. Nothing related to panels.

>> Third option, which is the best, but also something I have no idea how
>> to implement, would be to create the mgr-output links dynamically when
>> needed. The problem here is, of course, that a mgr could be allocated to
>> an output, only to be later found out that that particular mgr is needed
>> for another output.
>>
>> But this is something we could study a bit: can we create such mgr
>> allocation system, that no matter what outputs the board uses, it'll
>> just work.
> 
> Yes, that would be quite useful. But I think we'll hit situations where
> it is sort of impossible to prevent the above situation.
> 
> When an output needs a manager. We could study the current state of the
> system by splitting managers into 2 sets:
> 
> A: managers which already have outputs connected to them
> B: managers which don't have an output, but might get connected to one
> in the future.
> 
> managers in A are lost, and we can't detach them, we would need to at
> least disable/reenable the panel with a new manager connected to the
> output.
> 
> we need to find one from B such that maximum outputs(or some other
> weightage factor) will still be supported after this new link is made.
> 
> The system will initially have all managers in B, but eventually
> managers will move to A. We need to move one manager from B to A for
> every mgr-output link.
> 
> I guess I just described the problem in a more mathematical way, without
> providing any solution :), but it does look like an optimisation problem.

Well, optimization problem sounds like something that can always be
solved. But in this case the driver may need to predict what outputs
will be used, which is of course impossible.

>> So, for example, on omap4, LCD2 mgr can be used for DPI, DSI2, DSI1, and
>> RFBI. LCD1 can be used for RFBI and DSI1. If we know that DSI1 and RFBI
>> cannot be used at the same time, we're free to give LCD1 to either one.
> 
> But they can be used together, can't they? LCD1->DSI1 and LCD2->RFBI.
> Are you creating this constraint by assuming what the board is like? Or
> is this a constraint of OMAP DSS HW?

I didn't check if they can be used together or not, I was just guessing.
At least on OMAP3 DSI and RFBI shared the same pins, so they could not
be used at the same time.

Perhaps we should implement a mixed approach: the driver presumes
certain things, like "if DSI is used, RFBI is not used", based on the
knowledge of what kind of boards there currently are. This would allow
us to manage the mgr->output connections in the driver for, say, 95% of
the cases. Then we'd also have the platform data parameters for omapdss,
which could be used in the weird cases.

>> And if we know that DPI and DSI2 cannot be used at the same time, we're
>> also free to give LCD2 to either one. And if that's the case, there are
>> no conflicts.
> 
> This is also possible at the same time: TV->DPI and LCD2->DSI2

True. I was just again guessing. On OMAP3 DPI and DSI shared the same pins.

Thinking about this, OMAP4 does have separate pins for DSI, doesn't it?
So my guesses don't hold.

>> I don't know if we can find such allocation for all current omaps, and I
>> know it's slightly risky as the next omap could have limitations even if
>> current ones do not.
> 
> I don't understand the example so well, but I get your point of taking
> advantage of such limitations.
> 
>>
>>> Also, we would need to do this for omapdrm separately using it's own
>>> encoder/connector entities.
>>
>> Yep. That's also a negative side: both omapfb/omapdrm will need to
>> implement the same stuff, even if neither of them are really interested
>> in that stuff.
> 
> Yes. I wonder if crtcs, encoders and connectors already have some sort
> of helpers for this?

Probably nothing that helps us, as this is OMAP HW restriction.

 Tomi
archit taneja Dec. 10, 2012, 10:53 a.m. UTC | #5
On Monday 10 December 2012 03:37 PM, Tomi Valkeinen wrote:
> On 2012-12-10 11:54, Archit Taneja wrote:
>> On Monday 10 December 2012 01:33 PM, Tomi Valkeinen wrote:
>
>>> Another option would be to pass information about mgr-output links from
>>> the board files (or DT data) to the omapdss driver, so that omapdss
>>> could setup them at probe time. With this option omapfb/omapdrm doesn't
>>> need to care about this, and it doesn't create load order restriction.
>>> But mgr-output links are something that I'd really like to handle inside
>>> the drivers, not something that needs to be passed via platform data.
>>
>> This would definitely make things simpler, but if this parameter is put
>> in a panel's DT, it would become omap specific. We could add this info
>> to the DT corresponding to omapdss.
>
> Yes, I meant it should be omapdss platform data. Nothing related to panels.

I think this is the easiest way out. We can have one parameter per 
output in DT. If we do come up with a way to implement the 3rd option 
below, we can always ignore those DT fields(assuming our implementation 
to give the same result). So in this way, we would just be deprecating a 
DT field in the future, and calculating it ourselves.

>
>>> Third option, which is the best, but also something I have no idea how
>>> to implement, would be to create the mgr-output links dynamically when
>>> needed. The problem here is, of course, that a mgr could be allocated to
>>> an output, only to be later found out that that particular mgr is needed
>>> for another output.
>>>
>>> But this is something we could study a bit: can we create such mgr
>>> allocation system, that no matter what outputs the board uses, it'll
>>> just work.
>>
>> Yes, that would be quite useful. But I think we'll hit situations where
>> it is sort of impossible to prevent the above situation.
>>
>> When an output needs a manager. We could study the current state of the
>> system by splitting managers into 2 sets:
>>
>> A: managers which already have outputs connected to them
>> B: managers which don't have an output, but might get connected to one
>> in the future.
>>
>> managers in A are lost, and we can't detach them, we would need to at
>> least disable/reenable the panel with a new manager connected to the
>> output.
>>
>> we need to find one from B such that maximum outputs(or some other
>> weightage factor) will still be supported after this new link is made.
>>
>> The system will initially have all managers in B, but eventually
>> managers will move to A. We need to move one manager from B to A for
>> every mgr-output link.
>>
>> I guess I just described the problem in a more mathematical way, without
>> providing any solution :), but it does look like an optimisation problem.
>
> Well, optimization problem sounds like something that can always be
> solved. But in this case the driver may need to predict what outputs
> will be used, which is of course impossible.
>
>>> So, for example, on omap4, LCD2 mgr can be used for DPI, DSI2, DSI1, and
>>> RFBI. LCD1 can be used for RFBI and DSI1. If we know that DSI1 and RFBI
>>> cannot be used at the same time, we're free to give LCD1 to either one.
>>
>> But they can be used together, can't they? LCD1->DSI1 and LCD2->RFBI.
>> Are you creating this constraint by assuming what the board is like? Or
>> is this a constraint of OMAP DSS HW?
>
> I didn't check if they can be used together or not, I was just guessing.
> At least on OMAP3 DSI and RFBI shared the same pins, so they could not
> be used at the same time.
>
> Perhaps we should implement a mixed approach: the driver presumes
> certain things, like "if DSI is used, RFBI is not used", based on the
> knowledge of what kind of boards there currently are. This would allow
> us to manage the mgr->output connections in the driver for, say, 95% of
> the cases. Then we'd also have the platform data parameters for omapdss,
> which could be used in the weird cases.

Let's just have platform data parameters :)

>
>>> And if we know that DPI and DSI2 cannot be used at the same time, we're
>>> also free to give LCD2 to either one. And if that's the case, there are
>>> no conflicts.
>>
>> This is also possible at the same time: TV->DPI and LCD2->DSI2
>
> True. I was just again guessing. On OMAP3 DPI and DSI shared the same pins.
>
> Thinking about this, OMAP4 does have separate pins for DSI, doesn't it?
> So my guesses don't hold.
>
>>> I don't know if we can find such allocation for all current omaps, and I
>>> know it's slightly risky as the next omap could have limitations even if
>>> current ones do not.
>>
>> I don't understand the example so well, but I get your point of taking
>> advantage of such limitations.
>>
>>>
>>>> Also, we would need to do this for omapdrm separately using it's own
>>>> encoder/connector entities.
>>>
>>> Yep. That's also a negative side: both omapfb/omapdrm will need to
>>> implement the same stuff, even if neither of them are really interested
>>> in that stuff.
>>
>> Yes. I wonder if crtcs, encoders and connectors already have some sort
>> of helpers for this?
>
> Probably nothing that helps us, as this is OMAP HW restriction.

If we do use DT/platform data, would we need to parse it in omapdrm to 
establish drm entities? Or do we rely on omapdss to parse the DT data 
and give the links to omapdrm?

Archit

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomi Valkeinen Dec. 10, 2012, 11:03 a.m. UTC | #6
On 2012-12-10 12:53, Archit Taneja wrote:
> On Monday 10 December 2012 03:37 PM, Tomi Valkeinen wrote:
>> On 2012-12-10 11:54, Archit Taneja wrote:
>>> On Monday 10 December 2012 01:33 PM, Tomi Valkeinen wrote:
>>
>>>> Another option would be to pass information about mgr-output links from
>>>> the board files (or DT data) to the omapdss driver, so that omapdss
>>>> could setup them at probe time. With this option omapfb/omapdrm doesn't
>>>> need to care about this, and it doesn't create load order restriction.
>>>> But mgr-output links are something that I'd really like to handle
>>>> inside
>>>> the drivers, not something that needs to be passed via platform data.
>>>
>>> This would definitely make things simpler, but if this parameter is put
>>> in a panel's DT, it would become omap specific. We could add this info
>>> to the DT corresponding to omapdss.
>>
>> Yes, I meant it should be omapdss platform data. Nothing related to
>> panels.
> 
> I think this is the easiest way out. We can have one parameter per
> output in DT. If we do come up with a way to implement the 3rd option
> below, we can always ignore those DT fields(assuming our implementation
> to give the same result). So in this way, we would just be deprecating a
> DT field in the future, and calculating it ourselves.

I would rather go the other way around: calculate it ourselves
(presuming it can be done for the current boards), and add the DT field
later if we come up with boards that won't work with the dynamic approach.

The reasons I'm not too happy with having the parameter in the DT data
is that:

- DT should be about describing the hardware connections between
components, but in this case it's dynamically configurable connection.

- We need to have the DT parameter for all cases, even if in 95% of
cases it's not really needed.

- We may never need the parameter, if we never get boards that require
funny setup.

> If we do use DT/platform data, would we need to parse it in omapdrm to
> establish drm entities? Or do we rely on omapdss to parse the DT data
> and give the links to omapdrm?

I think we should parse it in omapdss and setup the connections at
omapdss's probe. Then when omapfb/omapdrm starts, they can get
information about the connections from omapdss, and setup their entities
as they want.

So omapdss would setup the mgr->output->panel links, and they would be
ready for omapfb/omapdrm to use.

 Tomi
diff mbox

Patch

diff --git a/drivers/video/omap2/omapfb/omapfb-main.c b/drivers/video/omap2/omapfb/omapfb-main.c
index 1df973e..24739fc 100644
--- a/drivers/video/omap2/omapfb/omapfb-main.c
+++ b/drivers/video/omap2/omapfb/omapfb-main.c
@@ -2353,27 +2353,37 @@  static int omapfb_init_display(struct omapfb2_device *fbdev,
 }
 
 static int omapfb_init_connections(struct omapfb2_device *fbdev,
-		struct omap_dss_device *dssdev)
+		struct omap_dss_device *def_dssdev)
 {
 	int i, r;
-	struct omap_overlay_manager *mgr = NULL;
+	struct omap_overlay_manager *mgr;
 
-	for (i = 0; i < fbdev->num_managers; i++) {
-		mgr = fbdev->managers[i];
-
-		if (dssdev->channel == mgr->id)
-			break;
+	if (!def_dssdev->output) {
+		dev_err(fbdev->dev, "no output for the default display\n");
+		return -EINVAL;
 	}
 
-	if (i == fbdev->num_managers)
-		return -ENODEV;
+	for (i = 0; i < fbdev->num_displays; ++i) {
+		struct omap_dss_device *dssdev = fbdev->displays[i].dssdev;
+		struct omap_dss_output *out = dssdev->output;
 
-	if (mgr->output)
-		mgr->unset_output(mgr);
+		mgr = omap_dss_get_overlay_manager(dssdev->channel);
 
-	r = mgr->set_output(mgr, dssdev->output);
-	if (r)
-		return r;
+		if (!mgr || !out)
+			continue;
+
+		if (mgr->output)
+			mgr->unset_output(mgr);
+
+		mgr->set_output(mgr, out);
+	}
+
+	mgr = def_dssdev->output->manager;
+
+	if (!mgr) {
+		dev_err(fbdev->dev, "no ovl manager for the default display\n");
+		return -EINVAL;
+	}
 
 	for (i = 0; i < fbdev->num_overlays; i++) {
 		struct omap_overlay *ovl = fbdev->overlays[i];