Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-29 Thread Rob Clark
On Mon, Apr 28, 2014 at 9:08 AM, Ajay kumar ajayn...@gmail.com wrote:
 Daniel,

 On Sun, Apr 27, 2014 at 6:25 PM, Daniel Vetter dan...@ffwll.ch wrote:
 On Fri, Apr 25, 2014 at 8:17 AM, Ajay kumar ajayn...@gmail.com wrote:
 We can call panel_enable/disable at the right point. Even the bridge chips
 expect the same. So, I am not ok with combining the bridge and panel and
 calling the fxn pointers from crtc_helpers.
 So, either we leave it the way it is in this patch (call drm_panel
 functions at right points) or
 we don't use drm_panel to control the panel sequence and instead use few DT
 properties and regulators, inside the bridge chip driver.

 That wasn't really suggested, but instead the idea was to provide a
 default drm_bridge which wraps the drm_panel so that you could more
 easily chain them up.
 What do you mean by default drm_bridge?
 I just want to clear few things. Let me explain what I have understood:
 What I understand is that Rob wants me to create a common
 structure which wraps up drm_panel and drm_bridge into a single structure:
   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
 struct drm_bridge *bridge; /* optional */  == Really not sure why
 this is needed!
   };

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pb = to_panel_bridge(bridge);
 if (pb-bridge)
   pb-bridge-funcs-enable(pb-bridge);
 pb-panel-funcs-enable(pb-panel);
   }

 And now, the above function becomes a common function
 (also with an ordering issue btw panel and bridge).
 Where do we keep it? And, where do we call it from?

my proposal involves no change to crtc helpers.  The later variation
(with composite_bridge so you could choose the order) would have a
sequence along the lines of:

crtc helpers:
+ bridge_funcs-enable(): - composite_bridge_enable()
   + cbridge-b1-funcs-enable(): - ptn3460_bridge_enable()
   + cbridge-b2-funcs-enable(): - panel_bridge_enable();
   + pbridge-panel-funcs-enable(): - foo_panel_enable()

or if the order needs to be swapped you can use ptn3460_bridge as b2
and panel bridge as b1.

BR,
-R

 Also I'm not really happy that the bridge callbacks have been added to
 the drm helpers (I'd prefer if driver callbacks would bear that
 responsibility). But you can always create your own drm_bridge
 integration.
 Do you mean, the bridge calls should be moved out of common drm_helper
 functions and called from platform specific drivers instead?

 In any case your concern that drivers need to control
 when certain callbacks are called is shared by everyone here afaics.
 And I also don't see any issues with Rob's proposal in this regard.
 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch

 Please let me know if my understanding is correct and correct me if
 there is a misconception.

 Regards,
 Ajay Kumar
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-28 Thread Ajay kumar
Daniel,

On Sun, Apr 27, 2014 at 6:25 PM, Daniel Vetter dan...@ffwll.ch wrote:
 On Fri, Apr 25, 2014 at 8:17 AM, Ajay kumar ajayn...@gmail.com wrote:
 We can call panel_enable/disable at the right point. Even the bridge chips
 expect the same. So, I am not ok with combining the bridge and panel and
 calling the fxn pointers from crtc_helpers.
 So, either we leave it the way it is in this patch (call drm_panel
 functions at right points) or
 we don't use drm_panel to control the panel sequence and instead use few DT
 properties and regulators, inside the bridge chip driver.

 That wasn't really suggested, but instead the idea was to provide a
 default drm_bridge which wraps the drm_panel so that you could more
 easily chain them up.
What do you mean by default drm_bridge?
I just want to clear few things. Let me explain what I have understood:
What I understand is that Rob wants me to create a common
structure which wraps up drm_panel and drm_bridge into a single structure:
  struct drm_panel_bridge {
struct drm_bridge base;
struct drm_panel *panel;
struct drm_bridge *bridge; /* optional */  == Really not sure why
this is needed!
  };

  static void drm_panel_bridge_enable(struct drm_bridge *bridge)
  {
struct drm_panel_bridge *pb = to_panel_bridge(bridge);
if (pb-bridge)
  pb-bridge-funcs-enable(pb-bridge);
pb-panel-funcs-enable(pb-panel);
  }

And now, the above function becomes a common function
(also with an ordering issue btw panel and bridge).
Where do we keep it? And, where do we call it from?

 Also I'm not really happy that the bridge callbacks have been added to
 the drm helpers (I'd prefer if driver callbacks would bear that
 responsibility). But you can always create your own drm_bridge
 integration.
Do you mean, the bridge calls should be moved out of common drm_helper
functions and called from platform specific drivers instead?

 In any case your concern that drivers need to control
 when certain callbacks are called is shared by everyone here afaics.
 And I also don't see any issues with Rob's proposal in this regard.
 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch

Please let me know if my understanding is correct and correct me if
there is a misconception.

Regards,
Ajay Kumar
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-27 Thread Daniel Vetter
On Fri, Apr 25, 2014 at 8:17 AM, Ajay kumar ajayn...@gmail.com wrote:
 We can call panel_enable/disable at the right point. Even the bridge chips
 expect the same. So, I am not ok with combining the bridge and panel and
 calling the fxn pointers from crtc_helpers.
 So, either we leave it the way it is in this patch (call drm_panel
 functions at right points) or
 we don't use drm_panel to control the panel sequence and instead use few DT
 properties and regulators, inside the bridge chip driver.

That wasn't really suggested, but instead the idea was to provide a
default drm_bridge which wraps the drm_panel so that you could more
easily chain them up.

Also I'm not really happy that the bridge callbacks have been added to
the drm helpers (I'd prefer if driver callbacks would bear that
responsibility). But you can always create your own drm_bridge
integration. In any case your concern that drivers need to control
when certain callbacks are called is shared by everyone here afaics.
And I also don't see any issues with Rob's proposal in this regard.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-25 Thread Ajay kumar
On Thu, Apr 24, 2014 at 11:08 PM, Ajay kumar ajayn...@gmail.com wrote:
 On Thu, Apr 24, 2014 at 10:55 PM, Rob Clark robdcl...@gmail.com wrote:
 On Thu, Apr 24, 2014 at 12:55 PM, Ajay kumar ajayn...@gmail.com wrote:
 Rob,

 On Thu, Apr 24, 2014 at 9:41 PM, Rob Clark robdcl...@gmail.com wrote:
 On Wed, Apr 23, 2014 at 3:02 PM, Ajay kumar ajayn...@gmail.com wrote:
 Sorry for the previous reply,

 Here goes the full explaination:

 Rob,

 On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote:
 So what about, rather than adding drm_panel support to each bridge
 individually, we introduce a drm_panel_bridge (with a form of
 chaining).. ie:

   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
 struct drm_bridge *bridge; /* optional */
   };

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pb = to_panel_bridge(bridge);
 if (pb-bridge)
   pb-bridge-funcs-enable(pb-bridge);
 pb-panel-funcs-enable(pb-panel);
   }

 We cannot call them like this from crtc helpers in the order you 
 mentioned,
 since each individual bridge chip expects the panel controls at
 different places.
 I mean,
 -- sometimes panel controls needs to be done before bridge
 controls(ptn3460: before RST_N and PD_N)

 well, this is why bridge has pre-enable/enable/disable/post-disable
 hooks, so you can choose before or after..
 These calls are for a bridge to sync with the encoder calls.
 We might end up defining pre-enable/enable/disable/post-disable for a
 panel to sync
 with the bridge calls! I have explained below.

 -- sometimes in between the bridge controls (ps8622: one panel control
 before SLP_N and one after SLP_N)
 -- sometimes panel controls needs to be done after bridge controls.

 I am not convinced that a generic panel/bridge adapter is not
 possible.  Maybe we need more fine grained fxn ptr callbacks, although
 seems like pre+post should give you enough.  It would certainly be
 easier than having to add panel support in each individual bridge
 driver (which seems like it will certainly result that only certain
 combinations of panel+bridge actually work as intended)
 Ok. Consider this case:
 Currently, we have the sequence as bridge-pre_enable,
 encoder_enable, bridge-enable
 And, the bridge should obviously be enabled in bridge-pre_enable.
 Now, where do you choose to call panel-pre_enable?
 -- as the first step of bridge-pre_enable, before we pull up/down bridge 
 GPIOs.
 -- at the last step of bridge-pre_enable, after we pull up/down the
 bridge GPIOs.

 Ideally, PTN3460 expects it to be the second case, and PS8625 expects
 it to be the first case.
 So, we will end up adding pre_pre_enable, post_pre_enable to
 accomodate such scenarios.

 ok, well I think we can deal with this with a slight modification of
 my original proposal.  Split drm_panel_bridge into
 drm_composite_bridge and drm_panel_bridge:

   struct drm_composite_bridge {
 struct drm_bridge base;
 struct drm_bridge *b1, *b2;
   }

   static void drm_composite_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_composite_bridge *cbridge = to_composite_bridge(bridge);
 cbridge-b1-funcs-enable(cbridge-b1);
 cbridge-b2-funcs-enable(cbridge-b2);
   }

   .. and so on ..

   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
   }

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pbridge = to_panel_bridge(bridge);
 pbridge-panel-funcs-enable(pbridge-panel);
   }

   .. and so on..


 then in initialization, order can be managed like:

   if (panel_first)
 bridge = drm_composite_bridge_init(panel_bridge, actual_bridge)
   else
 bridge = drm_composite_bridge_init(actual_bridge, panel_bridge);

   possibly parametrize drm_panel_bridge if we need to map
 panel-enable() to bridge-pre_enable() vs bridge-enable(), etc..

 Well, this really does seems complex to me.
 Don't you think just having a drm_panel inside drm_bridge structure is
 sufficient?!
 And, make a call for pre_panel_enable and post_panel_enable around
 bridge-pre_enable..and so on.?
Adding more comments:
The beauty of drm_panel is in the flexibility which it provides.
We can call panel_enable/disable at the right point. Even the
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-25 Thread Ajay kumar
Sorry for the previous reply. Here goes the full explaination.

On Fri, Apr 25, 2014 at 11:40 AM, Ajay kumar ajayn...@gmail.com wrote:
 On Thu, Apr 24, 2014 at 11:08 PM, Ajay kumar ajayn...@gmail.com wrote:
 On Thu, Apr 24, 2014 at 10:55 PM, Rob Clark robdcl...@gmail.com wrote:
 On Thu, Apr 24, 2014 at 12:55 PM, Ajay kumar ajayn...@gmail.com wrote:
 Rob,

 On Thu, Apr 24, 2014 at 9:41 PM, Rob Clark robdcl...@gmail.com wrote:
 On Wed, Apr 23, 2014 at 3:02 PM, Ajay kumar ajayn...@gmail.com wrote:
 Sorry for the previous reply,

 Here goes the full explaination:

 Rob,

 On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote:
 So what about, rather than adding drm_panel support to each bridge
 individually, we introduce a drm_panel_bridge (with a form of
 chaining).. ie:

   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
 struct drm_bridge *bridge; /* optional */
   };

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pb = to_panel_bridge(bridge);
 if (pb-bridge)
   pb-bridge-funcs-enable(pb-bridge);
 pb-panel-funcs-enable(pb-panel);
   }

 We cannot call them like this from crtc helpers in the order you 
 mentioned,
 since each individual bridge chip expects the panel controls at
 different places.
 I mean,
 -- sometimes panel controls needs to be done before bridge
 controls(ptn3460: before RST_N and PD_N)

 well, this is why bridge has pre-enable/enable/disable/post-disable
 hooks, so you can choose before or after..
 These calls are for a bridge to sync with the encoder calls.
 We might end up defining pre-enable/enable/disable/post-disable for a
 panel to sync
 with the bridge calls! I have explained below.

 -- sometimes in between the bridge controls (ps8622: one panel control
 before SLP_N and one after SLP_N)
 -- sometimes panel controls needs to be done after bridge controls.

 I am not convinced that a generic panel/bridge adapter is not
 possible.  Maybe we need more fine grained fxn ptr callbacks, although
 seems like pre+post should give you enough.  It would certainly be
 easier than having to add panel support in each individual bridge
 driver (which seems like it will certainly result that only certain
 combinations of panel+bridge actually work as intended)
 Ok. Consider this case:
 Currently, we have the sequence as bridge-pre_enable,
 encoder_enable, bridge-enable
 And, the bridge should obviously be enabled in bridge-pre_enable.
 Now, where do you choose to call panel-pre_enable?
 -- as the first step of bridge-pre_enable, before we pull up/down bridge 
 GPIOs.
 -- at the last step of bridge-pre_enable, after we pull up/down the
 bridge GPIOs.

 Ideally, PTN3460 expects it to be the second case, and PS8625 expects
 it to be the first case.
 So, we will end up adding pre_pre_enable, post_pre_enable to
 accomodate such scenarios.

 ok, well I think we can deal with this with a slight modification of
 my original proposal.  Split drm_panel_bridge into
 drm_composite_bridge and drm_panel_bridge:

   struct drm_composite_bridge {
 struct drm_bridge base;
 struct drm_bridge *b1, *b2;
   }

   static void drm_composite_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_composite_bridge *cbridge = to_composite_bridge(bridge);
 cbridge-b1-funcs-enable(cbridge-b1);
 cbridge-b2-funcs-enable(cbridge-b2);
   }

   .. and so on ..

   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
   }

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pbridge = to_panel_bridge(bridge);
 pbridge-panel-funcs-enable(pbridge-panel);
   }

   .. and so on..


 then in initialization, order can be managed like:

   if (panel_first)
 bridge = drm_composite_bridge_init(panel_bridge, actual_bridge)
   else
 bridge = drm_composite_bridge_init(actual_bridge, panel_bridge);

   possibly parametrize drm_panel_bridge if we need to map
 panel-enable() to bridge-pre_enable() vs bridge-enable(), etc..

 Well, this really does seems complex to me.
 Don't you think just having a drm_panel inside drm_bridge structure is
 sufficient?!
 And, make a call for pre_panel_enable and post_panel_enable around
 bridge-pre_enable..and so on.?
Adding more comments:
The beauty of drm_panel is in the flexibility which it provides.
We can call panel_enable/disable at the right point. Even the bridge chips
expect the same. So, I am not ok with combining the bridge and panel and
calling the fxn pointers from crtc_helpers.
So, either we leave it the way it is in this patch (call drm_panel
functions at right points) or
we don't use drm_panel to control the panel sequence and instead use few DT
properties and regulators, inside the bridge chip driver.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-24 Thread Rob Clark
On Wed, Apr 23, 2014 at 3:02 PM, Ajay kumar ajayn...@gmail.com wrote:
 Sorry for the previous reply,

 Here goes the full explaination:

 Rob,

 On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote:
 So what about, rather than adding drm_panel support to each bridge
 individually, we introduce a drm_panel_bridge (with a form of
 chaining).. ie:

   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
 struct drm_bridge *bridge; /* optional */
   };

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pb = to_panel_bridge(bridge);
 if (pb-bridge)
   pb-bridge-funcs-enable(pb-bridge);
 pb-panel-funcs-enable(pb-panel);
   }

 We cannot call them like this from crtc helpers in the order you mentioned,
 since each individual bridge chip expects the panel controls at
 different places.
 I mean,
 -- sometimes panel controls needs to be done before bridge
 controls(ptn3460: before RST_N and PD_N)

well, this is why bridge has pre-enable/enable/disable/post-disable
hooks, so you can choose before or after..

 -- sometimes in between the bridge controls (ps8622: one panel control
 before SLP_N and one after SLP_N)
 -- sometimes panel controls needs to be done after bridge controls.

I am not convinced that a generic panel/bridge adapter is not
possible.  Maybe we need more fine grained fxn ptr callbacks, although
seems like pre+post should give you enough.  It would certainly be
easier than having to add panel support in each individual bridge
driver (which seems like it will certainly result that only certain
combinations of panel+bridge actually work as intended)

 So, putting these drm_panel controls inside individual bridge drivers will 
 work,
 but keeping them in crtc_helpers might break stuff.

I'm not talking about crtc changing helpers.

BR,
-R

 Thanks and regards,
 Ajay Kumar
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-24 Thread Ajay kumar
Rob,

On Thu, Apr 24, 2014 at 9:41 PM, Rob Clark robdcl...@gmail.com wrote:
 On Wed, Apr 23, 2014 at 3:02 PM, Ajay kumar ajayn...@gmail.com wrote:
 Sorry for the previous reply,

 Here goes the full explaination:

 Rob,

 On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote:
 So what about, rather than adding drm_panel support to each bridge
 individually, we introduce a drm_panel_bridge (with a form of
 chaining).. ie:

   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
 struct drm_bridge *bridge; /* optional */
   };

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pb = to_panel_bridge(bridge);
 if (pb-bridge)
   pb-bridge-funcs-enable(pb-bridge);
 pb-panel-funcs-enable(pb-panel);
   }

 We cannot call them like this from crtc helpers in the order you mentioned,
 since each individual bridge chip expects the panel controls at
 different places.
 I mean,
 -- sometimes panel controls needs to be done before bridge
 controls(ptn3460: before RST_N and PD_N)

 well, this is why bridge has pre-enable/enable/disable/post-disable
 hooks, so you can choose before or after..
These calls are for a bridge to sync with the encoder calls.
We might end up defining pre-enable/enable/disable/post-disable for a
panel to sync
with the bridge calls! I have explained below.

 -- sometimes in between the bridge controls (ps8622: one panel control
 before SLP_N and one after SLP_N)
 -- sometimes panel controls needs to be done after bridge controls.

 I am not convinced that a generic panel/bridge adapter is not
 possible.  Maybe we need more fine grained fxn ptr callbacks, although
 seems like pre+post should give you enough.  It would certainly be
 easier than having to add panel support in each individual bridge
 driver (which seems like it will certainly result that only certain
 combinations of panel+bridge actually work as intended)
Ok. Consider this case:
Currently, we have the sequence as bridge-pre_enable,
encoder_enable, bridge-enable
And, the bridge should obviously be enabled in bridge-pre_enable.
Now, where do you choose to call panel-pre_enable?
-- as the first step of bridge-pre_enable, before we pull up/down bridge GPIOs.
-- at the last step of bridge-pre_enable, after we pull up/down the
bridge GPIOs.

Ideally, PTN3460 expects it to be the second case, and PS8625 expects
it to be the first case.
So, we will end up adding pre_pre_enable, post_pre_enable to
accomodate such scenarios.

So, we leave the choice for the individual bridge chip drivers to
implement panel
power up/down controls in the place where they wish to.


Thanks and regards,
 Ajay Kumar
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-24 Thread Rob Clark
On Thu, Apr 24, 2014 at 12:55 PM, Ajay kumar ajayn...@gmail.com wrote:
 Rob,

 On Thu, Apr 24, 2014 at 9:41 PM, Rob Clark robdcl...@gmail.com wrote:
 On Wed, Apr 23, 2014 at 3:02 PM, Ajay kumar ajayn...@gmail.com wrote:
 Sorry for the previous reply,

 Here goes the full explaination:

 Rob,

 On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote:
 So what about, rather than adding drm_panel support to each bridge
 individually, we introduce a drm_panel_bridge (with a form of
 chaining).. ie:

   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
 struct drm_bridge *bridge; /* optional */
   };

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pb = to_panel_bridge(bridge);
 if (pb-bridge)
   pb-bridge-funcs-enable(pb-bridge);
 pb-panel-funcs-enable(pb-panel);
   }

 We cannot call them like this from crtc helpers in the order you mentioned,
 since each individual bridge chip expects the panel controls at
 different places.
 I mean,
 -- sometimes panel controls needs to be done before bridge
 controls(ptn3460: before RST_N and PD_N)

 well, this is why bridge has pre-enable/enable/disable/post-disable
 hooks, so you can choose before or after..
 These calls are for a bridge to sync with the encoder calls.
 We might end up defining pre-enable/enable/disable/post-disable for a
 panel to sync
 with the bridge calls! I have explained below.

 -- sometimes in between the bridge controls (ps8622: one panel control
 before SLP_N and one after SLP_N)
 -- sometimes panel controls needs to be done after bridge controls.

 I am not convinced that a generic panel/bridge adapter is not
 possible.  Maybe we need more fine grained fxn ptr callbacks, although
 seems like pre+post should give you enough.  It would certainly be
 easier than having to add panel support in each individual bridge
 driver (which seems like it will certainly result that only certain
 combinations of panel+bridge actually work as intended)
 Ok. Consider this case:
 Currently, we have the sequence as bridge-pre_enable,
 encoder_enable, bridge-enable
 And, the bridge should obviously be enabled in bridge-pre_enable.
 Now, where do you choose to call panel-pre_enable?
 -- as the first step of bridge-pre_enable, before we pull up/down bridge 
 GPIOs.
 -- at the last step of bridge-pre_enable, after we pull up/down the
 bridge GPIOs.

 Ideally, PTN3460 expects it to be the second case, and PS8625 expects
 it to be the first case.
 So, we will end up adding pre_pre_enable, post_pre_enable to
 accomodate such scenarios.

ok, well I think we can deal with this with a slight modification of
my original proposal.  Split drm_panel_bridge into
drm_composite_bridge and drm_panel_bridge:

  struct drm_composite_bridge {
struct drm_bridge base;
struct drm_bridge *b1, *b2;
  }

  static void drm_composite_bridge_enable(struct drm_bridge *bridge)
  {
struct drm_composite_bridge *cbridge = to_composite_bridge(bridge);
cbridge-b1-funcs-enable(cbridge-b1);
cbridge-b2-funcs-enable(cbridge-b2);
  }

  .. and so on ..

  struct drm_panel_bridge {
struct drm_bridge base;
struct drm_panel *panel;
  }

  static void drm_panel_bridge_enable(struct drm_bridge *bridge)
  {
struct drm_panel_bridge *pbridge = to_panel_bridge(bridge);
pbridge-panel-funcs-enable(pbridge-panel);
  }

  .. and so on..


then in initialization, order can be managed like:

  if (panel_first)
bridge = drm_composite_bridge_init(panel_bridge, actual_bridge)
  else
bridge = drm_composite_bridge_init(actual_bridge, panel_bridge);

  possibly parametrize drm_panel_bridge if we need to map
panel-enable() to bridge-pre_enable() vs bridge-enable(), etc..

BR,
-R

 So, we leave the choice for the individual bridge chip drivers to
 implement panel
 power up/down controls in the place where they wish to.


 Thanks and regards,
  Ajay Kumar
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-24 Thread Ajay kumar
On Thu, Apr 24, 2014 at 10:55 PM, Rob Clark robdcl...@gmail.com wrote:
 On Thu, Apr 24, 2014 at 12:55 PM, Ajay kumar ajayn...@gmail.com wrote:
 Rob,

 On Thu, Apr 24, 2014 at 9:41 PM, Rob Clark robdcl...@gmail.com wrote:
 On Wed, Apr 23, 2014 at 3:02 PM, Ajay kumar ajayn...@gmail.com wrote:
 Sorry for the previous reply,

 Here goes the full explaination:

 Rob,

 On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote:
 So what about, rather than adding drm_panel support to each bridge
 individually, we introduce a drm_panel_bridge (with a form of
 chaining).. ie:

   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
 struct drm_bridge *bridge; /* optional */
   };

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pb = to_panel_bridge(bridge);
 if (pb-bridge)
   pb-bridge-funcs-enable(pb-bridge);
 pb-panel-funcs-enable(pb-panel);
   }

 We cannot call them like this from crtc helpers in the order you mentioned,
 since each individual bridge chip expects the panel controls at
 different places.
 I mean,
 -- sometimes panel controls needs to be done before bridge
 controls(ptn3460: before RST_N and PD_N)

 well, this is why bridge has pre-enable/enable/disable/post-disable
 hooks, so you can choose before or after..
 These calls are for a bridge to sync with the encoder calls.
 We might end up defining pre-enable/enable/disable/post-disable for a
 panel to sync
 with the bridge calls! I have explained below.

 -- sometimes in between the bridge controls (ps8622: one panel control
 before SLP_N and one after SLP_N)
 -- sometimes panel controls needs to be done after bridge controls.

 I am not convinced that a generic panel/bridge adapter is not
 possible.  Maybe we need more fine grained fxn ptr callbacks, although
 seems like pre+post should give you enough.  It would certainly be
 easier than having to add panel support in each individual bridge
 driver (which seems like it will certainly result that only certain
 combinations of panel+bridge actually work as intended)
 Ok. Consider this case:
 Currently, we have the sequence as bridge-pre_enable,
 encoder_enable, bridge-enable
 And, the bridge should obviously be enabled in bridge-pre_enable.
 Now, where do you choose to call panel-pre_enable?
 -- as the first step of bridge-pre_enable, before we pull up/down bridge 
 GPIOs.
 -- at the last step of bridge-pre_enable, after we pull up/down the
 bridge GPIOs.

 Ideally, PTN3460 expects it to be the second case, and PS8625 expects
 it to be the first case.
 So, we will end up adding pre_pre_enable, post_pre_enable to
 accomodate such scenarios.

 ok, well I think we can deal with this with a slight modification of
 my original proposal.  Split drm_panel_bridge into
 drm_composite_bridge and drm_panel_bridge:

   struct drm_composite_bridge {
 struct drm_bridge base;
 struct drm_bridge *b1, *b2;
   }

   static void drm_composite_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_composite_bridge *cbridge = to_composite_bridge(bridge);
 cbridge-b1-funcs-enable(cbridge-b1);
 cbridge-b2-funcs-enable(cbridge-b2);
   }

   .. and so on ..

   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
   }

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pbridge = to_panel_bridge(bridge);
 pbridge-panel-funcs-enable(pbridge-panel);
   }

   .. and so on..


 then in initialization, order can be managed like:

   if (panel_first)
 bridge = drm_composite_bridge_init(panel_bridge, actual_bridge)
   else
 bridge = drm_composite_bridge_init(actual_bridge, panel_bridge);

   possibly parametrize drm_panel_bridge if we need to map
 panel-enable() to bridge-pre_enable() vs bridge-enable(), etc..

Well, this really does seems complex to me.
Don't you think just having a drm_panel inside drm_bridge structure is
sufficient?!
And, make a call for pre_panel_enable and post_panel_enable around
bridge-pre_enable..and so on.?
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-23 Thread Ajay kumar
Rob,

On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote:
 So what about, rather than adding drm_panel support to each bridge
 individually, we introduce a drm_panel_bridge (with a form of
 chaining).. ie:

   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
 struct drm_bridge *bridge; /* optional */
   };

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pb = to_panel_bridge(bridge);
 if (pb-bridge)
   pb-bridge-funcs-enable(pb-bridge);
 pb-panel-funcs-enable(pb-panel);
   }

We cannot call them like this from crtc helpers in the order you mentioned,
since each individual bridge chip expects the panel controls at
different places.
I mean,
sometimes panel controls needs to be done before bridge controls,
 ... and so on.

 If you don't need a real bridge, just create one of these w/
 pb-bridge==NULL, otherwise create it as a wrapper for your real
 bridge.

 BR,
 -R

 On Mon, Apr 21, 2014 at 6:39 PM, Ajay Kumar ajaykumar...@samsung.com wrote:
 attach ptn3460 connector to drm_panel and support drm_panel routines,
 if a valid drm_panel object is passed to ptn3460_init.

 Signed-off-by: Ajay Kumar ajaykumar...@samsung.com
 ---
 Changes since V1:
 Address few coding style comments from Jingoo Han

  drivers/gpu/drm/bridge/Kconfig  |1 +
  drivers/gpu/drm/bridge/ptn3460.c|   20 +++-
  drivers/gpu/drm/exynos/exynos_dp_core.c |   16 
  include/drm/bridge/ptn3460.h|6 --
  4 files changed, 36 insertions(+), 7 deletions(-)

 diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
 index 884923f..3bc6845 100644
 --- a/drivers/gpu/drm/bridge/Kconfig
 +++ b/drivers/gpu/drm/bridge/Kconfig
 @@ -2,4 +2,5 @@ config DRM_PTN3460
 tristate PTN3460 DP/LVDS bridge
 depends on DRM
 select DRM_KMS_HELPER
 +   select DRM_PANEL
 ---help---
 diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
 b/drivers/gpu/drm/bridge/ptn3460.c
 index f1d2afc..3920202 100644
 --- a/drivers/gpu/drm/bridge/ptn3460.c
 +++ b/drivers/gpu/drm/bridge/ptn3460.c
 @@ -19,6 +19,7 @@
  #include linux/i2c.h
  #include linux/gpio.h
  #include linux/delay.h
 +#include drm/drm_panel.h

  #include drmP.h
  #include drm_edid.h
 @@ -38,6 +39,7 @@ struct ptn3460_bridge {
 struct i2c_client *client;
 struct drm_encoder *encoder;
 struct drm_bridge *bridge;
 +   struct drm_panel *panel;
 struct edid *edid;
 int gpio_pd_n;
 int gpio_rst_n;
 @@ -126,6 +128,8 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
 gpio_set_value(ptn_bridge-gpio_rst_n, 1);
 }

 +   drm_panel_pre_enable(ptn_bridge-panel);
 +
 /*
  * There's a bug in the PTN chip where it falsely asserts hotplug 
 before
  * it is fully functional. We're forced to wait for the maximum 
 start up
 @@ -142,6 +146,10 @@ static void ptn3460_pre_enable(struct drm_bridge 
 *bridge)

  static void ptn3460_enable(struct drm_bridge *bridge)
  {
 +   struct ptn3460_bridge *ptn_bridge = bridge-driver_private;
 +
 +   if (ptn_bridge-enabled)
 +   drm_panel_enable(ptn_bridge-panel);
  }

  static void ptn3460_disable(struct drm_bridge *bridge)
 @@ -153,6 +161,9 @@ static void ptn3460_disable(struct drm_bridge *bridge)

 ptn_bridge-enabled = false;

 +   drm_panel_disable(ptn_bridge-panel);
 +   drm_panel_post_disable(ptn_bridge-panel);
 +
 if (gpio_is_valid(ptn_bridge-gpio_rst_n))
 gpio_set_value(ptn_bridge-gpio_rst_n, 1);

 @@ -198,6 +209,7 @@ int ptn3460_get_modes(struct drm_connector *connector)

 power_off = !ptn_bridge-enabled;
 ptn3460_pre_enable(ptn_bridge-bridge);
 +   ptn3460_enable(ptn_bridge-bridge);

 edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
 if (!edid) {
 @@ -265,7 +277,8 @@ struct drm_connector_funcs ptn3460_connector_funcs = {
  };

  int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
 -   struct i2c_client *client, struct device_node *node)
 +   struct i2c_client *client, struct device_node *node,
 +   struct drm_panel *panel)
  {
 int ret;
 struct drm_bridge *bridge;
 @@ -324,6 +337,11 @@ int ptn3460_init(struct drm_device *dev, struct 
 drm_encoder *encoder,
 goto err;
 }

 +   if (panel) {
 +   ptn_bridge-panel = panel;
 +   drm_panel_attach(ptn_bridge-panel, ptn_bridge-connector);
 +   }
 +
 bridge-driver_private = ptn_bridge;
 encoder-bridge = bridge;
 ptn_bridge-connector.polled = DRM_CONNECTOR_POLL_HPD;
 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index dbc5ccc..4853f31 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ 

Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-23 Thread Ajay kumar
Sorry for the previous reply,

Here goes the full explaination:

 Rob,

 On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote:
 So what about, rather than adding drm_panel support to each bridge
 individually, we introduce a drm_panel_bridge (with a form of
 chaining).. ie:

   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
 struct drm_bridge *bridge; /* optional */
   };

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pb = to_panel_bridge(bridge);
 if (pb-bridge)
   pb-bridge-funcs-enable(pb-bridge);
 pb-panel-funcs-enable(pb-panel);
   }

We cannot call them like this from crtc helpers in the order you mentioned,
since each individual bridge chip expects the panel controls at
different places.
I mean,
-- sometimes panel controls needs to be done before bridge
controls(ptn3460: before RST_N and PD_N)
-- sometimes in between the bridge controls (ps8622: one panel control
before SLP_N and one after SLP_N)
-- sometimes panel controls needs to be done after bridge controls.

So, putting these drm_panel controls inside individual bridge drivers will work,
but keeping them in crtc_helpers might break stuff.

Thanks and regards,
Ajay Kumar
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-22 Thread Rob Clark
So what about, rather than adding drm_panel support to each bridge
individually, we introduce a drm_panel_bridge (with a form of
chaining).. ie:

  struct drm_panel_bridge {
struct drm_bridge base;
struct drm_panel *panel;
struct drm_bridge *bridge; /* optional */
  };

  static void drm_panel_bridge_enable(struct drm_bridge *bridge)
  {
struct drm_panel_bridge *pb = to_panel_bridge(bridge);
if (pb-bridge)
  pb-bridge-funcs-enable(pb-bridge);
pb-panel-funcs-enable(pb-panel);
  }

... and so on.

If you don't need a real bridge, just create one of these w/
pb-bridge==NULL, otherwise create it as a wrapper for your real
bridge.

BR,
-R

On Mon, Apr 21, 2014 at 6:39 PM, Ajay Kumar ajaykumar...@samsung.com wrote:
 attach ptn3460 connector to drm_panel and support drm_panel routines,
 if a valid drm_panel object is passed to ptn3460_init.

 Signed-off-by: Ajay Kumar ajaykumar...@samsung.com
 ---
 Changes since V1:
 Address few coding style comments from Jingoo Han

  drivers/gpu/drm/bridge/Kconfig  |1 +
  drivers/gpu/drm/bridge/ptn3460.c|   20 +++-
  drivers/gpu/drm/exynos/exynos_dp_core.c |   16 
  include/drm/bridge/ptn3460.h|6 --
  4 files changed, 36 insertions(+), 7 deletions(-)

 diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
 index 884923f..3bc6845 100644
 --- a/drivers/gpu/drm/bridge/Kconfig
 +++ b/drivers/gpu/drm/bridge/Kconfig
 @@ -2,4 +2,5 @@ config DRM_PTN3460
 tristate PTN3460 DP/LVDS bridge
 depends on DRM
 select DRM_KMS_HELPER
 +   select DRM_PANEL
 ---help---
 diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
 b/drivers/gpu/drm/bridge/ptn3460.c
 index f1d2afc..3920202 100644
 --- a/drivers/gpu/drm/bridge/ptn3460.c
 +++ b/drivers/gpu/drm/bridge/ptn3460.c
 @@ -19,6 +19,7 @@
  #include linux/i2c.h
  #include linux/gpio.h
  #include linux/delay.h
 +#include drm/drm_panel.h

  #include drmP.h
  #include drm_edid.h
 @@ -38,6 +39,7 @@ struct ptn3460_bridge {
 struct i2c_client *client;
 struct drm_encoder *encoder;
 struct drm_bridge *bridge;
 +   struct drm_panel *panel;
 struct edid *edid;
 int gpio_pd_n;
 int gpio_rst_n;
 @@ -126,6 +128,8 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
 gpio_set_value(ptn_bridge-gpio_rst_n, 1);
 }

 +   drm_panel_pre_enable(ptn_bridge-panel);
 +
 /*
  * There's a bug in the PTN chip where it falsely asserts hotplug 
 before
  * it is fully functional. We're forced to wait for the maximum start 
 up
 @@ -142,6 +146,10 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)

  static void ptn3460_enable(struct drm_bridge *bridge)
  {
 +   struct ptn3460_bridge *ptn_bridge = bridge-driver_private;
 +
 +   if (ptn_bridge-enabled)
 +   drm_panel_enable(ptn_bridge-panel);
  }

  static void ptn3460_disable(struct drm_bridge *bridge)
 @@ -153,6 +161,9 @@ static void ptn3460_disable(struct drm_bridge *bridge)

 ptn_bridge-enabled = false;

 +   drm_panel_disable(ptn_bridge-panel);
 +   drm_panel_post_disable(ptn_bridge-panel);
 +
 if (gpio_is_valid(ptn_bridge-gpio_rst_n))
 gpio_set_value(ptn_bridge-gpio_rst_n, 1);

 @@ -198,6 +209,7 @@ int ptn3460_get_modes(struct drm_connector *connector)

 power_off = !ptn_bridge-enabled;
 ptn3460_pre_enable(ptn_bridge-bridge);
 +   ptn3460_enable(ptn_bridge-bridge);

 edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
 if (!edid) {
 @@ -265,7 +277,8 @@ struct drm_connector_funcs ptn3460_connector_funcs = {
  };

  int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
 -   struct i2c_client *client, struct device_node *node)
 +   struct i2c_client *client, struct device_node *node,
 +   struct drm_panel *panel)
  {
 int ret;
 struct drm_bridge *bridge;
 @@ -324,6 +337,11 @@ int ptn3460_init(struct drm_device *dev, struct 
 drm_encoder *encoder,
 goto err;
 }

 +   if (panel) {
 +   ptn_bridge-panel = panel;
 +   drm_panel_attach(ptn_bridge-panel, ptn_bridge-connector);
 +   }
 +
 bridge-driver_private = ptn_bridge;
 encoder-bridge = bridge;
 ptn_bridge-connector.polled = DRM_CONNECTOR_POLL_HPD;
 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index dbc5ccc..4853f31 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
 @@ -989,13 +989,14 @@ static bool find_bridge(const char *compat, struct 
 bridge_init *bridge)

  /* returns the number of bridges attached */
  static int exynos_drm_attach_lcd_bridge(struct drm_device *dev,
 -   struct drm_encoder *encoder)
 +   struct drm_encoder *encoder, struct 

Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-22 Thread Daniel Vetter
On Tue, Apr 22, 2014 at 07:34:03AM -0400, Rob Clark wrote:
 So what about, rather than adding drm_panel support to each bridge
 individually, we introduce a drm_panel_bridge (with a form of
 chaining).. ie:
 
   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
 struct drm_bridge *bridge; /* optional */
   };
 
   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pb = to_panel_bridge(bridge);
 if (pb-bridge)
   pb-bridge-funcs-enable(pb-bridge);
 pb-panel-funcs-enable(pb-panel);
   }
 
 ... and so on.
 
 If you don't need a real bridge, just create one of these w/
 pb-bridge==NULL, otherwise create it as a wrapper for your real
 bridge.

Yeah I think that's how I'd have implemented some generic abstraction for
drivers using the crtc helpers. This allows you to keep bridge drivers,
panel drivers and anything else you might have in your driver to feed the
pixel stream to those 2 guys separate. And it also allows you to set it
all up in different ways, e.g. using device tree metadata, or acpi or some
other table hardcoded in a video rom somewhere.

Imo we also should have something similar to chain up drm_bridge devices.
tbh I'm not terribly happy about how the current integration with the crtc
modeset helpers is done ...
-Daniel

 
 BR,
 -R
 
 On Mon, Apr 21, 2014 at 6:39 PM, Ajay Kumar ajaykumar...@samsung.com wrote:
  attach ptn3460 connector to drm_panel and support drm_panel routines,
  if a valid drm_panel object is passed to ptn3460_init.
 
  Signed-off-by: Ajay Kumar ajaykumar...@samsung.com
  ---
  Changes since V1:
  Address few coding style comments from Jingoo Han
 
   drivers/gpu/drm/bridge/Kconfig  |1 +
   drivers/gpu/drm/bridge/ptn3460.c|   20 +++-
   drivers/gpu/drm/exynos/exynos_dp_core.c |   16 
   include/drm/bridge/ptn3460.h|6 --
   4 files changed, 36 insertions(+), 7 deletions(-)
 
  diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
  index 884923f..3bc6845 100644
  --- a/drivers/gpu/drm/bridge/Kconfig
  +++ b/drivers/gpu/drm/bridge/Kconfig
  @@ -2,4 +2,5 @@ config DRM_PTN3460
  tristate PTN3460 DP/LVDS bridge
  depends on DRM
  select DRM_KMS_HELPER
  +   select DRM_PANEL
  ---help---
  diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
  b/drivers/gpu/drm/bridge/ptn3460.c
  index f1d2afc..3920202 100644
  --- a/drivers/gpu/drm/bridge/ptn3460.c
  +++ b/drivers/gpu/drm/bridge/ptn3460.c
  @@ -19,6 +19,7 @@
   #include linux/i2c.h
   #include linux/gpio.h
   #include linux/delay.h
  +#include drm/drm_panel.h
 
   #include drmP.h
   #include drm_edid.h
  @@ -38,6 +39,7 @@ struct ptn3460_bridge {
  struct i2c_client *client;
  struct drm_encoder *encoder;
  struct drm_bridge *bridge;
  +   struct drm_panel *panel;
  struct edid *edid;
  int gpio_pd_n;
  int gpio_rst_n;
  @@ -126,6 +128,8 @@ static void ptn3460_pre_enable(struct drm_bridge 
  *bridge)
  gpio_set_value(ptn_bridge-gpio_rst_n, 1);
  }
 
  +   drm_panel_pre_enable(ptn_bridge-panel);
  +
  /*
   * There's a bug in the PTN chip where it falsely asserts hotplug 
  before
   * it is fully functional. We're forced to wait for the maximum 
  start up
  @@ -142,6 +146,10 @@ static void ptn3460_pre_enable(struct drm_bridge 
  *bridge)
 
   static void ptn3460_enable(struct drm_bridge *bridge)
   {
  +   struct ptn3460_bridge *ptn_bridge = bridge-driver_private;
  +
  +   if (ptn_bridge-enabled)
  +   drm_panel_enable(ptn_bridge-panel);
   }
 
   static void ptn3460_disable(struct drm_bridge *bridge)
  @@ -153,6 +161,9 @@ static void ptn3460_disable(struct drm_bridge *bridge)
 
  ptn_bridge-enabled = false;
 
  +   drm_panel_disable(ptn_bridge-panel);
  +   drm_panel_post_disable(ptn_bridge-panel);
  +
  if (gpio_is_valid(ptn_bridge-gpio_rst_n))
  gpio_set_value(ptn_bridge-gpio_rst_n, 1);
 
  @@ -198,6 +209,7 @@ int ptn3460_get_modes(struct drm_connector *connector)
 
  power_off = !ptn_bridge-enabled;
  ptn3460_pre_enable(ptn_bridge-bridge);
  +   ptn3460_enable(ptn_bridge-bridge);
 
  edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
  if (!edid) {
  @@ -265,7 +277,8 @@ struct drm_connector_funcs ptn3460_connector_funcs = {
   };
 
   int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
  -   struct i2c_client *client, struct device_node *node)
  +   struct i2c_client *client, struct device_node *node,
  +   struct drm_panel *panel)
   {
  int ret;
  struct drm_bridge *bridge;
  @@ -324,6 +337,11 @@ int ptn3460_init(struct drm_device *dev, struct 
  drm_encoder *encoder,
  goto err;
  }
 
  +   if (panel) {
  +  

[PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-21 Thread Ajay Kumar
attach ptn3460 connector to drm_panel and support drm_panel routines,
if a valid drm_panel object is passed to ptn3460_init.

Signed-off-by: Ajay Kumar ajaykumar...@samsung.com
---
Changes since V1:
Address few coding style comments from Jingoo Han

 drivers/gpu/drm/bridge/Kconfig  |1 +
 drivers/gpu/drm/bridge/ptn3460.c|   20 +++-
 drivers/gpu/drm/exynos/exynos_dp_core.c |   16 
 include/drm/bridge/ptn3460.h|6 --
 4 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 884923f..3bc6845 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -2,4 +2,5 @@ config DRM_PTN3460
tristate PTN3460 DP/LVDS bridge
depends on DRM
select DRM_KMS_HELPER
+   select DRM_PANEL
---help---
diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c
index f1d2afc..3920202 100644
--- a/drivers/gpu/drm/bridge/ptn3460.c
+++ b/drivers/gpu/drm/bridge/ptn3460.c
@@ -19,6 +19,7 @@
 #include linux/i2c.h
 #include linux/gpio.h
 #include linux/delay.h
+#include drm/drm_panel.h
 
 #include drmP.h
 #include drm_edid.h
@@ -38,6 +39,7 @@ struct ptn3460_bridge {
struct i2c_client *client;
struct drm_encoder *encoder;
struct drm_bridge *bridge;
+   struct drm_panel *panel;
struct edid *edid;
int gpio_pd_n;
int gpio_rst_n;
@@ -126,6 +128,8 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
gpio_set_value(ptn_bridge-gpio_rst_n, 1);
}
 
+   drm_panel_pre_enable(ptn_bridge-panel);
+
/*
 * There's a bug in the PTN chip where it falsely asserts hotplug before
 * it is fully functional. We're forced to wait for the maximum start up
@@ -142,6 +146,10 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
 
 static void ptn3460_enable(struct drm_bridge *bridge)
 {
+   struct ptn3460_bridge *ptn_bridge = bridge-driver_private;
+
+   if (ptn_bridge-enabled)
+   drm_panel_enable(ptn_bridge-panel);
 }
 
 static void ptn3460_disable(struct drm_bridge *bridge)
@@ -153,6 +161,9 @@ static void ptn3460_disable(struct drm_bridge *bridge)
 
ptn_bridge-enabled = false;
 
+   drm_panel_disable(ptn_bridge-panel);
+   drm_panel_post_disable(ptn_bridge-panel);
+
if (gpio_is_valid(ptn_bridge-gpio_rst_n))
gpio_set_value(ptn_bridge-gpio_rst_n, 1);
 
@@ -198,6 +209,7 @@ int ptn3460_get_modes(struct drm_connector *connector)
 
power_off = !ptn_bridge-enabled;
ptn3460_pre_enable(ptn_bridge-bridge);
+   ptn3460_enable(ptn_bridge-bridge);
 
edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
if (!edid) {
@@ -265,7 +277,8 @@ struct drm_connector_funcs ptn3460_connector_funcs = {
 };
 
 int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
-   struct i2c_client *client, struct device_node *node)
+   struct i2c_client *client, struct device_node *node,
+   struct drm_panel *panel)
 {
int ret;
struct drm_bridge *bridge;
@@ -324,6 +337,11 @@ int ptn3460_init(struct drm_device *dev, struct 
drm_encoder *encoder,
goto err;
}
 
+   if (panel) {
+   ptn_bridge-panel = panel;
+   drm_panel_attach(ptn_bridge-panel, ptn_bridge-connector);
+   }
+
bridge-driver_private = ptn_bridge;
encoder-bridge = bridge;
ptn_bridge-connector.polled = DRM_CONNECTOR_POLL_HPD;
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
b/drivers/gpu/drm/exynos/exynos_dp_core.c
index dbc5ccc..4853f31 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -989,13 +989,14 @@ static bool find_bridge(const char *compat, struct 
bridge_init *bridge)
 
 /* returns the number of bridges attached */
 static int exynos_drm_attach_lcd_bridge(struct drm_device *dev,
-   struct drm_encoder *encoder)
+   struct drm_encoder *encoder, struct drm_panel *panel)
 {
struct bridge_init bridge;
int ret;
 
if (find_bridge(nxp,ptn3460, bridge)) {
-   ret = ptn3460_init(dev, encoder, bridge.client, bridge.node);
+   ret = ptn3460_init(dev, encoder, bridge.client, bridge.node,
+   panel);
if (!ret)
return 1;
}
@@ -1012,9 +1013,16 @@ static int exynos_dp_create_connector(struct 
exynos_drm_display *display,
dp-encoder = encoder;
 
/* Pre-empt DP connector creation if there's a bridge */
-   ret = exynos_drm_attach_lcd_bridge(dp-drm_dev, encoder);
-   if (ret)
+   ret = exynos_drm_attach_lcd_bridge(dp-drm_dev, encoder, dp-drm_panel);
+   if (ret) {
+   /*
+* Also set dp-drm_panel = NULL so that we