date:20210525

Re: [Freedreno] [RFC PATCH 00/13] drm/msm: Add Display Stream Compression Support

2021-05-25 Thread Vinod Koul

Hello Jeff,

On 21-05-21, 08:09, Jeffrey Hugo wrote:
> On Fri, May 21, 2021 at 6:50 AM Vinod Koul  wrote:
> >
> > Display Stream Compression (DSC) compresses the display stream in host which
> > is later decoded by panel. This series enables this for Qualcomm msm driver.
> > This was tested on Google Pixel3 phone which use LGE SW43408 panel.
> >
> > The changes include adding DT properties for DSC then hardware blocks 
> > support
> > required in DPU1 driver and support in encoder. We also add support in DSI
> > and introduce required topology changes.
> >
> > In order for panel to set the DSC parameters we add dsc in drm_panel and set
> > it from the msm driver.
> >
> > Complete changes which enable this for Pixel3 along with panel driver (not
> > part of this series) and DT changes can be found at:
> > git.linaro.org/people/vinod.koul/kernel.git pixel/dsc_rfc
> >
> > Comments welcome!
> 
> This feels backwards to me.  I've only skimmed this series, and the DT
> changes didn't come through for me, so perhaps I have an incomplete
> view.

Not sure why, I see it on lore:
https://lore.kernel.org/dri-devel/20210521124946.3617862-3-vk...@kernel.org/

> DSC is not MSM specific.  There is a standard for it.  Yet it looks
> like everything is implemented in a MSM specific way, and then pushed
> to the panel.  So, every vendor needs to implement their vendor
> specific way to get the DSC info, and then push it to the panel?
> Seems wrong, given there is an actual standard for this feature.

I have added slice and bpp info in the DT here under the host and then
pass the generic struct drm_dsc_config to panel which allows panel to
write the pps cmd

Nothing above is MSM specific.. It can very well work with non MSM
controllers too.

I didn't envision DSC to be a specific thing, most of
the patches here are hardware enabling ones for DSC bits for MSM
hardware.

> Additionally, we define panel properties (resolution, BPP, etc) at the
> panel, and have the display drivers pull it from the panel.  However,
> for DSC, you do the reverse (define it in the display driver, and push
> it to the panel).  If the argument is that DSC properties can be
> dynamic, well, so can resolution.  Every panel for MSM MTPs supports
> multiple resolutions, yet we define that with the panel in Linux.

I dont have an answer for that right now, to start with yes the
properties are in host but I am okay to discuss this and put wherever we
feel is most correct thing.  I somehow dont like that we should pull
from panel DT and program host with that. Here using struct
drm_dsc_config allows me to configure panel based on resolution passed

> Finally, I haven't seen the DT bits, but I'm concerned about using DT
> for this.  It inherently excludes ACPI systems.  You appear to have
> sdm845 support in this series, but what about ACPI boot on the Lenovo
> C630 for example?  Or any of the 8cx laptops?  We don't read the panel
> resolution, etc from DT, so why the DSC?

But you must read from somewhere like ACPI tables. I think ACPI systems
would have some ACPI table info out there which would help on this.
Yes that is another task which we need to start with once we enable OF
systems.

Thanks
-- 
~Vinod
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [RFC PATCH 02/13] dt-bindings: msm/dsi: Document Display Stream Compression (DSC) parameters

2021-05-25 Thread Vinod Koul

On 24-05-21, 10:08, Bjorn Andersson wrote:
> On Mon 24 May 02:30 CDT 2021, Vinod Koul wrote:
> 
> > On 21-05-21, 09:42, Bjorn Andersson wrote:
> > > On Fri 21 May 07:49 CDT 2021, Vinod Koul wrote:
> > > 
> > > > DSC enables streams to be compressed before we send to panel. This
> > > > requires DSC enabled encoder and a panel to be present. So we add this
> > > > information in board DTS and find if DSC can be enabled and the
> > > > parameters required to configure DSC are added to binding document along
> > > > with example
> > > > 
> > > > Signed-off-by: Vinod Koul 
> > > > ---
> > > >  .../devicetree/bindings/display/msm/dsi.txt   | 15 +++
> > > >  1 file changed, 15 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt 
> > > > b/Documentation/devicetree/bindings/display/msm/dsi.txt
> > > > index b9a64d3ff184..83d2fb92267e 100644
> > > > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
> > > > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
> > > > @@ -48,6 +48,13 @@ Optional properties:
> > > >  - pinctrl-n: the "sleep" pinctrl state
> > > >  - ports: contains DSI controller input and output ports as children, 
> > > > each
> > > >containing one endpoint subnode.
> > > > +- qcom,mdss-dsc-enabled: Display Stream Compression (DSC) is enabled
> > > > +- qcom,mdss-slice-height: DSC slice height in pixels
> > > > +- qcom,mdss-slice-width: DSC slice width in pixels
> > > > +- qcom,mdss-slice-per-pkt: DSC slices per packet
> > > > +- qcom,mdss-bit-per-component: DSC bits per component
> > > > +- qcom,mdss-bit-per-pixel: DSC bits per pixel
> > > > +- qcom,mdss-block-prediction-enable: Block prediction mode of DSC 
> > > > enabled
> > > >  
> > > >DSI Endpoint properties:
> > > >- remote-endpoint: For port@0, set to phandle of the connected 
> > > > panel/bridge's
> > > > @@ -188,6 +195,14 @@ Example:
> > > > qcom,master-dsi;
> > > > qcom,sync-dual-dsi;
> > > >  
> > > > +   qcom,mdss-dsc-enabled;
> > > 
> > > To me the activation of DSC seems to be a property of the panel.
> > 
> > I think there are three parts to the problem
> > 1. Panel needs to support it
> 
> In the case of DP there's bits to be read in the panel to figure this
> out, for DSI panels this seems like a property that the panel (driver)
> should know about.

Yes panel should know..

> 
> > 2. Host needs to support it
> 
> Right, so this needs to be known by the driver. My suggestion is that we
> derive it from the compatible or from the HW version.

Okay

> 
> > 3. Someone needs to decide to use when both the above conditions are
> > met.
> > 
> > There are cases where above 1, 2 will be satisfied, but we might be okay
> > without DSC too.. so how to decide when to do DSC :)
> > 
> 
> Can we describe those cases? E.g. is it because enabling DSC would not
> cause a reduction in clock speed that's worth the effort? Or do we only
> use DSC for DSI when it allows us to squeeze everything into a single
> link?

I really dont know how and when we should decide that :-|
One thing we can do is that if both support then always enable and get
benefit of getting power and clock speed reduction.

With this, who should have slice and bpp settings, panel or host?

Thanks
-- 
~Vinod
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH v2 4/6] drm/msm/dpu: replace IRQ lookup with the data in hw catalog

2021-05-25 Thread Dmitry Baryshkov


On 25/05/2021 00:57, abhin...@codeaurora.org wrote:

On 2021-05-16 13:29, Dmitry Baryshkov wrote:

The IRQ table in the dpu_hw_interrupts.h is big, ugly, and hard to
maintain. There are only few interrupts used from that table. Newer
generations use different IRQ locations. Move this data to hw catalog.

I think you can drop the line that "only few interrupts ..." are used as 
that
was specific to sc7280 as you mentioned on IRC or give the example of 
sc7280

here to explain that as being one of the motivations for this cleanup.


I've had this before sc7280 (but it intensified the necessity for these 
changes). Regarding "only few", e.g. DSPP, VIG, PP_WR_PTR, 
PP_AUTOREFRESH interrupts are not used.





Signed-off-by: Dmitry Baryshkov 


With that fixed, I think this cleanup looks quite reasonable to me.
I computed a few of the intr offsets and the values were matching the 
prev hard-coded
offsets. Since I dont have the sc7280 HW to test this, I am checking if 
someone with that
hw in our team can test it out as well to make sure that it covers that 
hw as well.


But otherwise,

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c  |  20 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h  |  13 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   |  64 +++-
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |   2 -
 .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  |  36 ++---
 .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  |  31 +---
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    | 150 +++---
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |  12 +-
 .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 137 +++-
 .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h |  17 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h |  28 ++--
 11 files changed, 229 insertions(+), 281 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
index fd11a2aeab6c..11c0abed21ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
@@ -43,16 +43,6 @@ static void dpu_core_irq_callback_handler(void
*arg, int irq_idx)
 spin_unlock_irqrestore(_kms->irq_obj.cb_lock, irq_flags);
 }

-int dpu_core_irq_idx_lookup(struct dpu_kms *dpu_kms,
-    enum dpu_intr_type intr_type, u32 instance_idx)
-{
-    if (!dpu_kms->hw_intr || !dpu_kms->hw_intr->ops.irq_idx_lookup)
-    return -EINVAL;
-
-    return dpu_kms->hw_intr->ops.irq_idx_lookup(dpu_kms->hw_intr,
-    intr_type, instance_idx);
-}
-
 /**
  * _dpu_core_irq_enable - enable core interrupt given by the index
  * @dpu_kms:    Pointer to dpu kms context
@@ -70,7 +60,7 @@ static int _dpu_core_irq_enable(struct dpu_kms
*dpu_kms, int irq_idx)
 return -EINVAL;
 }

-    if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->irq_idx_tbl_size) {
+    if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->total_irqs) {
 DPU_ERROR("invalid IRQ index: [%d]\n", irq_idx);
 return -EINVAL;
 }
@@ -133,7 +123,7 @@ static int _dpu_core_irq_disable(struct dpu_kms
*dpu_kms, int irq_idx)
 return -EINVAL;
 }

-    if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->irq_idx_tbl_size) {
+    if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->total_irqs) {
 DPU_ERROR("invalid IRQ index: [%d]\n", irq_idx);
 return -EINVAL;
 }
@@ -208,7 +198,7 @@ int dpu_core_irq_register_callback(struct dpu_kms
*dpu_kms, int irq_idx,
 return -EINVAL;
 }

-    if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->irq_idx_tbl_size) {
+    if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->total_irqs) {
 DPU_ERROR("invalid IRQ index: [%d]\n", irq_idx);
 return -EINVAL;
 }
@@ -243,7 +233,7 @@ int dpu_core_irq_unregister_callback(struct
dpu_kms *dpu_kms, int irq_idx,
 return -EINVAL;
 }

-    if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->irq_idx_tbl_size) {
+    if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->total_irqs) {
 DPU_ERROR("invalid IRQ index: [%d]\n", irq_idx);
 return -EINVAL;
 }
@@ -328,7 +318,7 @@ void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms)
 spin_lock_init(_kms->irq_obj.cb_lock);

 /* Create irq callbacks for all possible irq_idx */
-    dpu_kms->irq_obj.total_irqs = dpu_kms->hw_intr->irq_idx_tbl_size;
+    dpu_kms->irq_obj.total_irqs = dpu_kms->hw_intr->total_irqs;
 dpu_kms->irq_obj.irq_cb_tbl = kcalloc(dpu_kms->irq_obj.total_irqs,
 sizeof(struct list_head), GFP_KERNEL);
 dpu_kms->irq_obj.enable_counts = 
kcalloc(dpu_kms->irq_obj.total_irqs,

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
index e30775e6585b..d147784d5531 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
@@ -29,19 +29,6 @@ void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms);
  */
 irqreturn_t dpu_core_irq(struct dpu_kms *dpu_kms);

Re: [Freedreno] [PATCH] drm/msm/disp/dpu1/dpu_encoder: Drop unnecessary NULL checks after container_of

2021-05-25 Thread Dmitry Baryshkov


On 25/05/2021 14:29, Guenter Roeck wrote:

The result of container_of() operations is never NULL unless the embedded
element is the first element of the structure. This is not the case here.
The NULL checks on the result of container_of() are therefore unnecessary
and misleading. Remove them.

This change was made automatically with the following Coccinelle script.

@@
type t;
identifier v;
statement s;
@@

<+...
(
   t v = container_of(...);
|
   v = container_of(...);
)
   ...
   when != v
- if (\( !v \| v == NULL \) ) s
...+>

Signed-off-by: Guenter Roeck 



Reviewed-by: Dmitry Baryshkov 


---
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 --
  1 file changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 8d942052db8a..a573fe211375 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1453,11 +1453,6 @@ static void dpu_encoder_off_work(struct work_struct 
*work)
struct dpu_encoder_virt *dpu_enc = container_of(work,
struct dpu_encoder_virt, delayed_off_work.work);
  
-	if (!dpu_enc) {

-   DPU_ERROR("invalid dpu encoder\n");
-   return;
-   }
-
dpu_encoder_resource_control(_enc->base,
DPU_ENC_RC_EVENT_ENTER_IDLE);
  
@@ -1797,11 +1792,6 @@ static void dpu_encoder_vsync_event_work_handler(struct kthread_work *work)

struct dpu_encoder_virt, vsync_event_work);
ktime_t wakeup_time;
  
-	if (!dpu_enc) {

-   DPU_ERROR("invalid dpu encoder\n");
-   return;
-   }
-
if (dpu_encoder_vsync_time(_enc->base, _time))
return;
  




--
With best wishes
Dmitry
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH] drm/msm: remove unneeded variable ret

2021-05-25 Thread Dmitry Baryshkov


On 07/04/2021 16:06, Bernard Zhao wrote:

This patch fix coccicheck warning:
drivers/gpu/drm/msm/dp/dp_link.c:848:5-8: Unneeded variable: "ret". Return "0" 
on line 880
Also remove unneeded function return value check.

Signed-off-by: Bernard Zhao 


Reviewed-by: Dmitry Baryshkov 



---
  drivers/gpu/drm/msm/dp/dp_link.c | 15 +++
  1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
index be986da78c4a..3395b08155a6 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.c
+++ b/drivers/gpu/drm/msm/dp/dp_link.c
@@ -843,10 +843,8 @@ bool dp_link_send_edid_checksum(struct dp_link *dp_link, 
u8 checksum)
return ret == 1;
  }
  
-static int dp_link_parse_vx_px(struct dp_link_private *link)

+static void dp_link_parse_vx_px(struct dp_link_private *link)
  {
-   int ret = 0;
-
DRM_DEBUG_DP("vx: 0=%d, 1=%d, 2=%d, 3=%d\n",
drm_dp_get_adjust_request_voltage(link->link_status, 0),
drm_dp_get_adjust_request_voltage(link->link_status, 1),
@@ -876,8 +874,6 @@ static int dp_link_parse_vx_px(struct dp_link_private *link)
DRM_DEBUG_DP("Requested: v_level = 0x%x, p_level = 0x%x\n",
link->dp_link.phy_params.v_level,
link->dp_link.phy_params.p_level);
-
-   return ret;
  }
  
  /**

@@ -891,8 +887,6 @@ static int dp_link_parse_vx_px(struct dp_link_private *link)
  static int dp_link_process_phy_test_pattern_request(
struct dp_link_private *link)
  {
-   int ret = 0;
-
if (!(link->request.test_requested & DP_TEST_LINK_PHY_TEST_PATTERN)) {
DRM_DEBUG_DP("no phy test\n");
return -EINVAL;
@@ -918,12 +912,9 @@ static int dp_link_process_phy_test_pattern_request(
link->dp_link.link_params.rate =
drm_dp_bw_code_to_link_rate(link->request.test_link_rate);
  
-	ret = dp_link_parse_vx_px(link);

-
-   if (ret)
-   DRM_ERROR("parse_vx_px failed. ret=%d\n", ret);
+   dp_link_parse_vx_px(link);
  
-	return ret;

+   return 0;
  }
  
  static u8 get_link_status(const u8 link_status[DP_LINK_STATUS_SIZE], int r)





--
With best wishes
Dmitry
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH] drm/msm/dp: remove the repeated declaration

2021-05-25 Thread abhinavk


On 2021-05-25 05:22, Shaokun Zhang wrote:

Function 'dp_catalog_audio_enable' is declared twice, remove the
repeated declaration.

Cc: Rob Clark 
Cc: Abhinav Kumar 
Signed-off-by: Shaokun Zhang 

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/dp/dp_catalog.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 176a9020a520..f12468dcbb56 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -124,7 +124,6 @@ void dp_catalog_audio_get_header(struct dp_catalog
*catalog);
 void dp_catalog_audio_set_header(struct dp_catalog *catalog);
 void dp_catalog_audio_config_acr(struct dp_catalog *catalog);
 void dp_catalog_audio_enable(struct dp_catalog *catalog);
-void dp_catalog_audio_enable(struct dp_catalog *catalog);
 void dp_catalog_audio_config_sdp(struct dp_catalog *catalog);
 void dp_catalog_audio_init(struct dp_catalog *catalog);
 void dp_catalog_audio_sfe_level(struct dp_catalog *catalog);

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH] drm/msm/dp: Drop unnecessary NULL checks after container_of

2021-05-25 Thread abhinavk


On 2021-05-24 20:20, Guenter Roeck wrote:
The result of container_of() operations is never NULL unless the 
embedded
element is the first element of the structure. This is not the case 
here.

The NULL check on the result of container_of() is therefore unnecessary
and misleading. Remove it.

This change was made automatically with the following Coccinelle 
script.


@@
type t;
identifier v;
statement s;
@@

<+...
(
  t v = container_of(...);
|
  v = container_of(...);
)
  ...
  when != v
- if (\( !v \| v == NULL \) ) s
...+>

While at it, remove unused but assigned variable hpd in
dp_display_usbpd_attention_cb().

Signed-off-by: Guenter Roeck 

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/dp/dp_display.c | 25 -
 1 file changed, 25 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 1784e119269b..a74e7ef96fcf 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -208,10 +208,6 @@ static int dp_display_bind(struct device *dev,
struct device *master,

dp = container_of(g_dp_display,
struct dp_display_private, dp_display);
-   if (!dp) {
-   DRM_ERROR("DP driver bind failed. Invalid driver data\n");
-   return -EINVAL;
-   }

dp->dp_display.drm_dev = drm;
priv = drm->dev_private;
@@ -252,10 +248,6 @@ static void dp_display_unbind(struct device *dev,
struct device *master,

dp = container_of(g_dp_display,
struct dp_display_private, dp_display);
-   if (!dp) {
-   DRM_ERROR("Invalid DP driver data\n");
-   return;
-   }

dp_power_client_deinit(dp->power);
dp_aux_unregister(dp->aux);
@@ -406,11 +398,6 @@ static int dp_display_usbpd_configure_cb(struct
device *dev)

dp = container_of(g_dp_display,
struct dp_display_private, dp_display);
-   if (!dp) {
-   DRM_ERROR("no driver data found\n");
-   rc = -ENODEV;
-   goto end;
-   }

dp_display_host_init(dp, false);

@@ -437,11 +424,6 @@ static int dp_display_usbpd_disconnect_cb(struct
device *dev)

dp = container_of(g_dp_display,
struct dp_display_private, dp_display);
-   if (!dp) {
-   DRM_ERROR("no driver data found\n");
-   rc = -ENODEV;
-   return rc;
-   }

dp_add_event(dp, EV_USER_NOTIFICATION, false, 0);

@@ -502,7 +484,6 @@ static int dp_display_usbpd_attention_cb(struct 
device *dev)

int rc = 0;
u32 sink_request;
struct dp_display_private *dp;
-   struct dp_usbpd *hpd;

if (!dev) {
DRM_ERROR("invalid dev\n");
@@ -511,12 +492,6 @@ static int dp_display_usbpd_attention_cb(struct
device *dev)

dp = container_of(g_dp_display,
struct dp_display_private, dp_display);
-   if (!dp) {
-   DRM_ERROR("no driver data found\n");
-   return -ENODEV;
-   }
-
-   hpd = dp->usbpd;

/* check for any test request issued by sink */
rc = dp_link_process_request(dp->link);

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [v4 1/4] drm/panel-simple: Add basic DPCD backlight support

2021-05-25 Thread Doug Anderson

Hi,

On Tue, May 25, 2021 at 12:31 AM Rajeev Nandan  wrote:
>
> @@ -171,6 +172,19 @@ struct panel_desc {
>
> /** @connector_type: LVDS, eDP, DSI, DPI, etc. */
> int connector_type;
> +
> +   /**
> +* @uses_dpcd_backlight: Panel supports eDP dpcd backlight control.
> +*
> +* Set true, if the panel supports backlight control over eDP AUX 
> channel
> +* using DPCD registers as per VESA's standard.
> +*/
> +   bool uses_dpcd_backlight;
> +};
> +
> +struct edp_backlight {
> +   struct backlight_device *dev;

Can you pick a name other than "dev". In my mind "dev" means you've
got a "struct device" or a "struct device *".


> +   struct drm_edp_backlight_info info;
>  };
>
>  struct panel_simple {
> @@ -194,6 +208,8 @@ struct panel_simple {
>
> struct edid *edid;
>
> +   struct edp_backlight *edp_bl;
> +

I don't think you need to add this pointer. See below for details, but
basically the backlight device should be in base.backlight. Any code
that needs the containing structure can use the standard
"container_of" syntax.


> struct drm_display_mode override_mode;
>
> enum drm_panel_orientation orientation;
> @@ -330,10 +346,14 @@ static void panel_simple_wait(ktime_t start_ktime, 
> unsigned int min_ms)
>  static int panel_simple_disable(struct drm_panel *panel)
>  {
> struct panel_simple *p = to_panel_simple(panel);
> +   struct edp_backlight *bl = p->edp_bl;
>
> if (!p->enabled)
> return 0;
>
> +   if (p->desc->uses_dpcd_backlight && bl)
> +   drm_edp_backlight_disable(p->aux, >info);
> +

It feels like this shouldn't be needed. I would have expected that
your backlight should be in 'panel->backlight'. Then
drm_panel_enable() will call backlight_enable() on your backlight
automatically after calling the panel's enable function.


> if (p->desc->delay.disable)
> msleep(p->desc->delay.disable);
>
> @@ -496,6 +516,7 @@ static int panel_simple_prepare(struct drm_panel *panel)
>  static int panel_simple_enable(struct drm_panel *panel)
>  {
> struct panel_simple *p = to_panel_simple(panel);
> +   struct edp_backlight *bl = p->edp_bl;
>
> if (p->enabled)
> return 0;
> @@ -505,6 +526,10 @@ static int panel_simple_enable(struct drm_panel *panel)
>
> panel_simple_wait(p->prepared_time, p->desc->delay.prepare_to_enable);
>
> +   if (p->desc->uses_dpcd_backlight && bl)
> +   drm_edp_backlight_enable(p->aux, >info,
> +bl->dev->props.brightness);
> +

Similar to disable, this shouldn't be needed.


> p->enabled = true;
>
> return 0;
> @@ -565,6 +590,59 @@ static const struct drm_panel_funcs panel_simple_funcs = 
> {
> .get_timings = panel_simple_get_timings,
>  };
>
> +static int edp_backlight_update_status(struct backlight_device *bd)
> +{
> +   struct panel_simple *p = bl_get_data(bd);
> +   struct edp_backlight *bl = p->edp_bl;
> +
> +   if (!p->enabled)
> +   return 0;
> +
> +   return drm_edp_backlight_set_level(p->aux, >info, 
> bd->props.brightness);

I notice that the "nouveau" driver grabs a whole pile of locks around
this. Do we need some of those? I guess perhaps checking "p->enabled"
isn't so valid without holding some of those locks.

Actually, I guess you probably can't look at "p->enabled" anyway if
this gets moved out of panel-simple as I'm suggesting.

...but do you even need something like this check? Shouldn't it be
handled by the fact that drm_panel will handle enabling/disabling the
backlight at the right times?


> +}
> +
> +static const struct backlight_ops edp_backlight_ops = {
> +   .update_status = edp_backlight_update_status,
> +};
> +
> +static int edp_backlight_register(struct device *dev, struct panel_simple 
> *panel)
> +{
> +   struct edp_backlight *bl;
> +   struct backlight_properties props = { 0 };
> +   u16 current_level;
> +   u8 current_mode;
> +   u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
> +   int ret;
> +
> +   bl = devm_kzalloc(dev, sizeof(*bl), GFP_KERNEL);
> +   if (!bl)
> +   return -ENOMEM;
> +
> +   ret = drm_dp_dpcd_read(panel->aux, DP_EDP_DPCD_REV, edp_dpcd,
> +  EDP_DISPLAY_CTL_CAP_SIZE);
> +   if (ret < 0)
> +   return ret;
> +
> +   ret = drm_edp_backlight_init(panel->aux, >info, 0, edp_dpcd,
> +_level, _mode);
> +   if (ret < 0)
> +   return ret;
> +
> +   props.type = BACKLIGHT_RAW;
> +   props.brightness = current_level;
> +   props.max_brightness = bl->info.max;
> +
> +   bl->dev = devm_backlight_device_register(dev, "edp_backlight",
> +   dev, panel,
> +   _backlight_ops, );
> +   if

Re: [Freedreno] [v4 3/4] dt-bindings: display: simple: Add Samsung ATNA33XC20

2021-05-25 Thread Doug Anderson

Hi,

On Tue, May 25, 2021 at 12:31 AM Rajeev Nandan  wrote:
>
> Add Samsung 13.3" FHD eDP AMOLED panel.
>
> Signed-off-by: Rajeev Nandan 
> ---
>
> Changes in v4:
> - New
>
>  Documentation/devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml 
> b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
> index 4a0a5e1..f5acfd6 100644
> --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
> @@ -242,6 +242,8 @@ properties:
>- rocktech,rk101ii01d-ct
>  # Rocktech Display Ltd. RK070ER9427 800(RGB)x480 TFT LCD panel
>- rocktech,rk070er9427
> +# Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel
> +  - samsung,atna33xc20
>  # Samsung 12.2" (2560x1600 pixels) TFT LCD panel
>- samsung,lsn122dl01-c01

This panel is slightly different from other panels currently listed
here because it requires the DP AUX channel to control the backlight.
However, in my mind, it still qualifies as "simple" because this fact
is probable and no extra dt properties are needed. Thus:

Reviewed-by: Douglas Anderson 
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [v4 4/4] drm/panel-simple: Add Samsung ATNA33XC20

2021-05-25 Thread Doug Anderson

Hi,

On Tue, May 25, 2021 at 12:31 AM Rajeev Nandan  wrote:
>
> Add Samsung 13.3" FHD eDP AMOLED panel.
>
> Signed-off-by: Rajeev Nandan 
> ---
>
> Changes in v4:
> - New
>
>  drivers/gpu/drm/panel/panel-simple.c | 34 ++
>  1 file changed, 34 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c 
> b/drivers/gpu/drm/panel/panel-simple.c
> index caed71b..21af794 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -3644,6 +3644,37 @@ static const struct panel_desc rocktech_rk101ii01d_ct 
> = {
> .connector_type = DRM_MODE_CONNECTOR_LVDS,
>  };
>
> +static const struct drm_display_mode samsung_atna33xc20_mode = {
> +   .clock = 138770,
> +   .hdisplay = 1920,
> +   .hsync_start = 1920 + 48,
> +   .hsync_end = 1920 + 48 + 32,
> +   .htotal = 1920 + 48 + 32 + 80,
> +   .vdisplay = 1080,
> +   .vsync_start = 1080 + 8,
> +   .vsync_end = 1080 + 8 + 8,
> +   .vtotal = 1080 + 8 + 8 + 16,
> +   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC,
> +};
> +
> +static const struct panel_desc samsung_atna33xc20 = {
> +   .modes = _atna33xc20_mode,
> +   .num_modes = 1,
> +   .bpc = 10,
> +   .size = {
> +   .width = 294,
> +   .height = 165,
> +   },
> +   .delay = {
> +   .disable_to_power_off = 150,
> +   .power_to_enable = 150,
> +   .hpd_absent_delay = 200,
> +   .unprepare = 500,
> +   },
> +   .connector_type = DRM_MODE_CONNECTOR_eDP,
> +   .uses_dpcd_backlight = true,

>From my feedback on the previous patch in this series, I believe the
"uses_dpcd_backlight" property should be removed and this should be
auto-detected. Other than that this patch looks fine to me. Feel free
to add my Reviewed-by tag next spin when that property is removed.
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [v4 2/4] drm/panel-simple: Support for delays between GPIO & regulator

2021-05-25 Thread Doug Anderson

Hi,

On Tue, May 25, 2021 at 12:31 AM Rajeev Nandan  wrote:
>
> Some panels datasheets may specify a delay between the enable GPIO and
> the regulator. Support this in panel-simple.
>
> Signed-off-by: Rajeev Nandan 
> ---
>
> Changes in v4:
> - New
>
>  drivers/gpu/drm/panel/panel-simple.c | 23 +++
>  1 file changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c 
> b/drivers/gpu/drm/panel/panel-simple.c
> index f9e4e60..caed71b 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -134,6 +134,22 @@ struct panel_desc {
> unsigned int prepare_to_enable;
>
> /**
> +* @delay.power_to_enable: Time for the power to enable the 
> display on.
> +*
> +* The time (in milliseconds) that it takes for the panel to
> +* turn the display on.

Maybe a slightly better description:

The time (in milliseconds) to wait after powering up the display
before asserting its enable pin.


> +*/
> +   unsigned int power_to_enable;
> +
> +   /**
> +* @delay.disable_to_power_off: Time for the disable to power 
> the display off.
> +*
> +* The time (in milliseconds) that it takes for the panel to
> +* turn the display off.

Maybe a slightly better description:

The time (in milliseconds) to wait after disabling the display before
deasserting its enable pin.


> +*/
> +   unsigned int disable_to_power_off;
> +
> +   /**
>  * @delay.enable: Time for the panel to display a valid frame.
>  *
>  * The time (in milliseconds) that it takes for the panel to
> @@ -367,6 +383,10 @@ static int panel_simple_suspend(struct device *dev)
> struct panel_simple *p = dev_get_drvdata(dev);
>
> gpiod_set_value_cansleep(p->enable_gpio, 0);
> +
> +   if (p->desc->delay.disable_to_power_off)
> +   msleep(p->desc->delay.disable_to_power_off);
> +

I wonder if it's worth a warning if
"p->desc->delay.disable_to_power_off" is non-zero and p->enable_gpio
is NULL? I guess in theory it'd also be nice to confirm that p->supply
wasn't a dummy regulator, but that's slightly harder.


> regulator_disable(p->supply);
> p->unprepared_time = ktime_get();
>
> @@ -427,6 +447,9 @@ static int panel_simple_prepare_once(struct panel_simple 
> *p)
> return err;
> }
>
> +   if (p->desc->delay.power_to_enable)
> +   msleep(p->desc->delay.power_to_enable);
> +

Similar to above: I wonder if it's worth a warning if
"p->desc->delay.power_to_enable" is non-zero and p->enable_gpio is
NULL?

-Doug
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH 7/7] drm/msm/mdp5: provide dynamic bandwidth management

2021-05-25 Thread kernel test robot

Hi Dmitry,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.13-rc3 next-20210525]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Dmitry-Baryshkov/drm-msm-mdp5-add-properties-and-bandwidth-management/20210525-211559
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
a050a6d2b7e80ca52b2f4141eaf3420d201b72b3
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# 
https://github.com/0day-ci/linux/commit/1077a27642ba4bcb15951c29bffdd94bfc378dbe
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Dmitry-Baryshkov/drm-msm-mdp5-add-properties-and-bandwidth-management/20210525-211559
git checkout 1077a27642ba4bcb15951c29bffdd94bfc378dbe
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c: In function 'mdp5_plane_calc_bw':
>> drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c:169:16: warning: variable 'vfp' 
>> set but not used [-Wunused-but-set-variable]
 169 |  int vbp, vpw, vfp;
 |^~~


vim +/vfp +169 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c

   156  
   157  /* based on _dpu_plane_calc_bw */
   158  static void mdp5_plane_calc_bw(struct drm_plane_state *state, struct 
drm_crtc_state *crtc_state)
   159  {
   160  struct drm_framebuffer *fb = state->fb;
   161  struct mdp5_plane_state *pstate = to_mdp5_plane_state(state);
   162  struct drm_display_mode *mode = _state->mode;
   163  int bpp;
   164  int src_width, src_height, dst_height, fps;
   165  u64 plane_bw;
   166  u32 hw_latency_lines;
   167  u32 prefill_div;
   168  u64 scale_factor;
 > 169  int vbp, vpw, vfp;
   170  
   171  src_width = drm_rect_width(>src) >> 16;
   172  src_height = drm_rect_height(>src) >> 16;
   173  dst_height = drm_rect_height(>dst);
   174  fps = drm_mode_vrefresh(mode);
   175  vbp = mode->vtotal - mode->vsync_end;
   176  vpw = mode->vsync_end - mode->vsync_start;
   177  vfp = mode->vsync_start - mode->vdisplay;
   178  scale_factor = src_height > dst_height ?
   179  mult_frac(src_height, 1, dst_height) : 1;
   180  
   181  bpp = to_mdp_format(msm_framebuffer_format(fb))->cpp;
   182  
   183  plane_bw = src_width * mode->vtotal * fps * bpp * scale_factor;
   184  
   185  hw_latency_lines = 21; /* or 24? */
   186  prefill_div = hw_latency_lines;
   187  if (vbp + vpw > hw_latency_lines)
   188  prefill_div = vbp + vpw;
   189  #if 0
   190  else if (vbp + vpw + vfp < hw_latency_lines)
   191  prefill_div = vbp + vpw + vfp;
   192  #endif
   193  
   194  pstate->plane_bw = max(plane_bw, mult_frac(plane_bw, 
hw_latency_lines, prefill_div));
   195  }
   196  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH 1/2] iommu/arm-smmu-qcom: Skip the TTBR1 quirk for db820c.

2021-05-25 Thread Bjorn Andersson

On Tue 30 Mar 10:31 CDT 2021, Will Deacon wrote:

> On Tue, Mar 30, 2021 at 08:03:36AM -0700, Rob Clark wrote:
> > On Tue, Mar 30, 2021 at 2:34 AM Will Deacon  wrote:
> > >
> > > On Mon, Mar 29, 2021 at 09:02:50PM -0700, Rob Clark wrote:
> > > > On Mon, Mar 29, 2021 at 7:47 AM Will Deacon  wrote:
> > > > >
> > > > > On Fri, Mar 26, 2021 at 04:13:02PM -0700, Eric Anholt wrote:
> > > > > > db820c wants to use the qcom smmu path to get HUPCF set (which keeps
> > > > > > the GPU from wedging and then sometimes wedging the kernel after a
> > > > > > page fault), but it doesn't have separate pagetables support yet in
> > > > > > drm/msm so we can't go all the way to the TTBR1 path.
> > > > >
> > > > > What do you mean by "doesn't have separate pagetables support yet"? 
> > > > > The
> > > > > compatible string doesn't feel like the right way to determine this.
> > > >
> > > > the compatible string identifies what it is, not what the sw
> > > > limitations are, so in that regard it seems right to me..
> > >
> > > Well it depends on what "doesn't have separate pagetables support yet"
> > > means. I can't tell if it's a hardware issue, a firmware issue or a driver
> > > issue.
> > 
> > Just a driver issue (and the fact that currently we don't have
> > physical access to a device... debugging a5xx per-process-pgtables by
> > pushing untested things to the CI farm is kind of a difficult way to
> > work)
> 
> But then in that case, this is using the compatible string to identify a
> driver issue, no?
> 

No the compatible addition identifies the hardware, the implementation
then uses this information to know that it needs to behave "differently"
on this platform.

When/if someone decides to add the necessary support in the driver they
can remove the driver quirk, but it doesn't invalidate the specific
compatible.

Regards,
Bjorn
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH] drm/msm/disp/dpu1/dpu_encoder: Drop unnecessary NULL checks after container_of

2021-05-25 Thread Bjorn Andersson

On Tue 25 May 06:29 CDT 2021, Guenter Roeck wrote:

> The result of container_of() operations is never NULL unless the embedded
> element is the first element of the structure. This is not the case here.
> The NULL checks on the result of container_of() are therefore unnecessary
> and misleading. Remove them.
> 
> This change was made automatically with the following Coccinelle script.
> 
> @@
> type t;
> identifier v;
> statement s;
> @@
> 
> <+...
> (
>   t v = container_of(...);
> |
>   v = container_of(...);
> )
>   ...
>   when != v
> - if (\( !v \| v == NULL \) ) s
> ...+>
> 

Reviewed-by: Bjorn Andersson 

> Signed-off-by: Guenter Roeck 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 --
>  1 file changed, 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 8d942052db8a..a573fe211375 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1453,11 +1453,6 @@ static void dpu_encoder_off_work(struct work_struct 
> *work)
>   struct dpu_encoder_virt *dpu_enc = container_of(work,
>   struct dpu_encoder_virt, delayed_off_work.work);
>  
> - if (!dpu_enc) {
> - DPU_ERROR("invalid dpu encoder\n");
> - return;
> - }
> -
>   dpu_encoder_resource_control(_enc->base,
>   DPU_ENC_RC_EVENT_ENTER_IDLE);
>  
> @@ -1797,11 +1792,6 @@ static void 
> dpu_encoder_vsync_event_work_handler(struct kthread_work *work)
>   struct dpu_encoder_virt, vsync_event_work);
>   ktime_t wakeup_time;
>  
> - if (!dpu_enc) {
> - DPU_ERROR("invalid dpu encoder\n");
> - return;
> - }
> -
>   if (dpu_encoder_vsync_time(_enc->base, _time))
>   return;
>  
> -- 
> 2.25.1
> 
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH] drm/msm/dp: remove the repeated declaration

2021-05-25 Thread Bjorn Andersson

On Tue 25 May 07:22 CDT 2021, Shaokun Zhang wrote:

> Function 'dp_catalog_audio_enable' is declared twice, remove the
> repeated declaration.
> 
> Cc: Rob Clark 
> Cc: Abhinav Kumar 

Reviewed-by: Bjorn Andersson 

> Signed-off-by: Shaokun Zhang 
> ---
>  drivers/gpu/drm/msm/dp/dp_catalog.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h 
> b/drivers/gpu/drm/msm/dp/dp_catalog.h
> index 176a9020a520..f12468dcbb56 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.h
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
> @@ -124,7 +124,6 @@ void dp_catalog_audio_get_header(struct dp_catalog 
> *catalog);
>  void dp_catalog_audio_set_header(struct dp_catalog *catalog);
>  void dp_catalog_audio_config_acr(struct dp_catalog *catalog);
>  void dp_catalog_audio_enable(struct dp_catalog *catalog);
> -void dp_catalog_audio_enable(struct dp_catalog *catalog);
>  void dp_catalog_audio_config_sdp(struct dp_catalog *catalog);
>  void dp_catalog_audio_init(struct dp_catalog *catalog);
>  void dp_catalog_audio_sfe_level(struct dp_catalog *catalog);
> -- 
> 2.7.4
> 
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH] drm/msm/dp: remove the repeated declaration

2021-05-25 Thread Shaokun Zhang

Function 'dp_catalog_audio_enable' is declared twice, remove the
repeated declaration.

Cc: Rob Clark 
Cc: Abhinav Kumar 
Signed-off-by: Shaokun Zhang 
---
 drivers/gpu/drm/msm/dp/dp_catalog.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h 
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 176a9020a520..f12468dcbb56 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -124,7 +124,6 @@ void dp_catalog_audio_get_header(struct dp_catalog 
*catalog);
 void dp_catalog_audio_set_header(struct dp_catalog *catalog);
 void dp_catalog_audio_config_acr(struct dp_catalog *catalog);
 void dp_catalog_audio_enable(struct dp_catalog *catalog);
-void dp_catalog_audio_enable(struct dp_catalog *catalog);
 void dp_catalog_audio_config_sdp(struct dp_catalog *catalog);
 void dp_catalog_audio_init(struct dp_catalog *catalog);
 void dp_catalog_audio_sfe_level(struct dp_catalog *catalog);
-- 
2.7.4

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 5/7] drm/msm/mdp5: switch to standard zpos property

2021-05-25 Thread Dmitry Baryshkov

Instead of implemeting zpos property on our own, use standard zpos
property support.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c  |   2 +-
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h   |   3 -
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 114 ++---
 3 files changed, 10 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index ed4d91420417..f482e0911d03 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -650,7 +650,7 @@ static int pstate_cmp(const void *a, const void *b)
 {
struct plane_state *pa = (struct plane_state *)a;
struct plane_state *pb = (struct plane_state *)b;
-   return pa->state->zpos - pb->state->zpos;
+   return pa->state->base.normalized_zpos - 
pb->state->base.normalized_zpos;
 }
 
 /* is there a helper for this? */
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
index d124c9bcdc60..ac269a6802df 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
@@ -98,9 +98,6 @@ struct mdp5_plane_state {
struct mdp5_hw_pipe *hwpipe;
struct mdp5_hw_pipe *r_hwpipe;  /* right hwpipe */
 
-   /* aligned with property */
-   uint8_t zpos;
-
/* assigned by crtc blender */
enum mdp_mixer_stage_id stage;
 };
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index 9c678e336e7a..c6b69afcbac8 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -44,8 +44,9 @@ static void mdp5_plane_destroy(struct drm_plane *plane)
kfree(mdp5_plane);
 }
 
-static void mdp5_plane_install_rotation_property(struct drm_device *dev,
-   struct drm_plane *plane)
+/* helper to install properties which are common to planes and crtcs */
+static void mdp5_plane_install_properties(struct drm_plane *plane,
+   struct drm_mode_object *obj)
 {
drm_plane_create_rotation_property(plane,
   DRM_MODE_ROTATE_0,
@@ -53,109 +54,12 @@ static void mdp5_plane_install_rotation_property(struct 
drm_device *dev,
   DRM_MODE_ROTATE_180 |
   DRM_MODE_REFLECT_X |
   DRM_MODE_REFLECT_Y);
-}
-
-/* helper to install properties which are common to planes and crtcs */
-static void mdp5_plane_install_properties(struct drm_plane *plane,
-   struct drm_mode_object *obj)
-{
-   struct drm_device *dev = plane->dev;
-   struct msm_drm_private *dev_priv = dev->dev_private;
-   struct drm_property *prop;
-
-#define INSTALL_PROPERTY(name, NAME, init_val, fnc, ...) do { \
-   prop = dev_priv->plane_property[PLANE_PROP_##NAME]; \
-   if (!prop) { \
-   prop = drm_property_##fnc(dev, 0, #name, \
-   ##__VA_ARGS__); \
-   if (!prop) { \
-   dev_warn(dev->dev, \
-   "Create property %s failed\n", \
-   #name); \
-   return; \
-   } \
-   dev_priv->plane_property[PLANE_PROP_##NAME] = prop; \
-   } \
-   drm_object_attach_property(>base, prop, init_val); \
-   } while (0)
-
-#define INSTALL_RANGE_PROPERTY(name, NAME, min, max, init_val) \
-   INSTALL_PROPERTY(name, NAME, init_val, \
-   create_range, min, max)
-
-#define INSTALL_ENUM_PROPERTY(name, NAME, init_val) \
-   INSTALL_PROPERTY(name, NAME, init_val, \
-   create_enum, name##_prop_enum_list, \
-   ARRAY_SIZE(name##_prop_enum_list))
-
-   INSTALL_RANGE_PROPERTY(zpos, ZPOS, 1, 255, 1);
-
-   mdp5_plane_install_rotation_property(dev, plane);
drm_plane_create_alpha_property(plane);
drm_plane_create_blend_mode_property(plane,
BIT(DRM_MODE_BLEND_PIXEL_NONE) |
BIT(DRM_MODE_BLEND_PREMULTI) |
BIT(DRM_MODE_BLEND_COVERAGE));
-
-#undef INSTALL_RANGE_PROPERTY
-#undef INSTALL_ENUM_PROPERTY
-#undef INSTALL_PROPERTY
-}
-
-static int mdp5_plane_atomic_set_property(struct drm_plane *plane,
-   struct drm_plane_state *state, struct drm_property *property,
-   uint64_t val)
-{
-   struct drm_device *dev = plane->dev;
-   struct mdp5_plane_state *pstate;
-   struct msm_drm_private *dev_priv = dev->dev_private;
-   int ret = 0;
-
-   pstate = to_mdp5_plane_state(state);
-
-#define SET_PROPERTY(name, NAME, type) do { \
-   if

[Freedreno] [PATCH 4/7] drm/msm/mdp5: add support for alpha/blend_mode properties

2021-05-25 Thread Dmitry Baryshkov

Hook alpha and pixel blend mode support to be exported as proper DRM
plane properties. This allows using this functionality from the
userspace.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index 90cd825df16b..9c678e336e7a 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -91,6 +91,11 @@ static void mdp5_plane_install_properties(struct drm_plane 
*plane,
INSTALL_RANGE_PROPERTY(zpos, ZPOS, 1, 255, 1);
 
mdp5_plane_install_rotation_property(dev, plane);
+   drm_plane_create_alpha_property(plane);
+   drm_plane_create_blend_mode_property(plane,
+   BIT(DRM_MODE_BLEND_PIXEL_NONE) |
+   BIT(DRM_MODE_BLEND_PREMULTI) |
+   BIT(DRM_MODE_BLEND_COVERAGE));
 
 #undef INSTALL_RANGE_PROPERTY
 #undef INSTALL_ENUM_PROPERTY
-- 
2.30.2

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 6/7] drm/msm/mdp5: add perf blocks for holding fudge factors

2021-05-25 Thread Dmitry Baryshkov

From: James Willcox 

Prior downstream kernels had "fudge factors" in devicetree which would
be applied to things like interconnect bandwidth calculations. Bring
some of those values back here.

Signed-off-by: James Willcox 
[DB: changed _ff to _inefficiency, fixed patch description]
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 35 
 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h |  7 +
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
index 94ce62a26daf..9741544ffc35 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
@@ -95,6 +95,11 @@ static const struct mdp5_cfg_hw msm8x74v1_config = {
[3] = INTF_HDMI,
},
},
+   .perf = {
+   .ab_inefficiency = 200,
+   .ib_inefficiency = 120,
+   .clk_inefficiency = 125
+   },
.max_clk = 2,
 };
 
@@ -177,6 +182,11 @@ static const struct mdp5_cfg_hw msm8x74v2_config = {
[3] = INTF_HDMI,
},
},
+   .perf = {
+   .ab_inefficiency = 200,
+   .ib_inefficiency = 120,
+   .clk_inefficiency = 125
+   },
.max_clk = 32000,
 };
 
@@ -272,6 +282,11 @@ static const struct mdp5_cfg_hw apq8084_config = {
[3] = INTF_HDMI,
},
},
+   .perf = {
+   .ab_inefficiency = 200,
+   .ib_inefficiency = 120,
+   .clk_inefficiency = 105
+   },
.max_clk = 32000,
 };
 
@@ -339,6 +354,11 @@ static const struct mdp5_cfg_hw msm8x16_config = {
[1] = INTF_DSI,
},
},
+   .perf = {
+   .ab_inefficiency = 100,
+   .ib_inefficiency = 200,
+   .clk_inefficiency = 105
+   },
.max_clk = 32000,
 };
 
@@ -414,6 +434,11 @@ static const struct mdp5_cfg_hw msm8x36_config = {
[2] = INTF_DSI,
},
},
+   .perf = {
+   .ab_inefficiency = 100,
+   .ib_inefficiency = 200,
+   .clk_inefficiency = 105
+   },
.max_clk = 36667,
 };
 
@@ -509,6 +534,11 @@ static const struct mdp5_cfg_hw msm8x94_config = {
[3] = INTF_HDMI,
},
},
+   .perf = {
+   .ab_inefficiency = 100,
+   .ib_inefficiency = 100,
+   .clk_inefficiency = 105
+   },
.max_clk = 4,
 };
 
@@ -617,6 +647,11 @@ static const struct mdp5_cfg_hw msm8x96_config = {
[3] = INTF_HDMI,
},
},
+   .perf = {
+   .ab_inefficiency = 100,
+   .ib_inefficiency = 200,
+   .clk_inefficiency = 105
+   },
.max_clk = 41250,
 };
 
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
index 1c50d01f15f5..6b03d7899309 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
@@ -76,6 +76,12 @@ struct mdp5_intf_block {
u32 connect[MDP5_INTF_NUM_MAX]; /* array of enum mdp5_intf_type */
 };
 
+struct mdp5_perf_block {
+   u32 ab_inefficiency;
+   u32 ib_inefficiency;
+   u32 clk_inefficiency;
+};
+
 struct mdp5_cfg_hw {
char  *name;
 
@@ -93,6 +99,7 @@ struct mdp5_cfg_hw {
struct mdp5_sub_block dsc;
struct mdp5_sub_block cdm;
struct mdp5_intf_block intf;
+   struct mdp5_perf_block perf;
 
uint32_t max_clk;
 };
-- 
2.30.2

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 7/7] drm/msm/mdp5: provide dynamic bandwidth management

2021-05-25 Thread Dmitry Baryshkov

Instead of using static bandwidth setup, manage bandwidth dynamically,
depending on the amount of allocated planes, their format and
resolution.

Co-developed-with: James Willcox 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c  |  44 
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c   | 119 ++---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h   |  12 +++
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c |  42 
 4 files changed, 181 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index f482e0911d03..a9332078aa13 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -43,6 +43,9 @@ struct mdp5_crtc {
/* for unref'ing cursor bo's after scanout completes: */
struct drm_flip_work unref_cursor_work;
 
+   /* for lowering down the bandwidth after previous frame is complete */
+   struct drm_flip_work lower_bw_work;
+
struct mdp_irq vblank;
struct mdp_irq err;
struct mdp_irq pp_done;
@@ -171,12 +174,28 @@ static void unref_cursor_worker(struct drm_flip_work 
*work, void *val)
drm_gem_object_put(val);
 }
 
+static void lower_bw_worker(struct drm_flip_work *work, void *val)
+{
+   struct mdp5_crtc *mdp5_crtc =
+   container_of(work, struct mdp5_crtc, lower_bw_work);
+   struct drm_crtc *crtc = _crtc->base;
+   struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc->state);
+   struct mdp5_kms *mdp5_kms = get_kms(_crtc->base);
+
+   if (mdp5_cstate->old_crtc_bw > mdp5_cstate->new_crtc_bw) {
+   DBG("DOWN BW to %lld\n", mdp5_cstate->new_crtc_bw);
+   mdp5_kms_set_bandwidth(mdp5_kms);
+   mdp5_cstate->old_crtc_bw = mdp5_cstate->new_crtc_bw;
+   }
+}
+
 static void mdp5_crtc_destroy(struct drm_crtc *crtc)
 {
struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
 
drm_crtc_cleanup(crtc);
drm_flip_work_cleanup(_crtc->unref_cursor_work);
+   drm_flip_work_cleanup(_crtc->lower_bw_work);
 
kfree(mdp5_crtc);
 }
@@ -691,6 +710,7 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state,
  crtc);
struct mdp5_kms *mdp5_kms = get_kms(crtc);
+   struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc_state);
struct drm_plane *plane;
struct drm_device *dev = crtc->dev;
struct plane_state pstates[STAGE_MAX + 1];
@@ -701,6 +721,7 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
bool need_right_mixer = false;
int cnt = 0, i;
int ret;
+   u64 crtc_bw = 0;
enum mdp_mixer_stage_id start;
 
DBG("%s: check", crtc->name);
@@ -718,6 +739,9 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
 */
if (pstates[cnt].state->r_hwpipe)
need_right_mixer = true;
+
+   crtc_bw += pstates[cnt].state->plane_bw;
+
cnt++;
 
if (plane->type == DRM_PLANE_TYPE_CURSOR)
@@ -730,6 +754,10 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
 
hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
 
+   if (hw_cfg->perf.ab_inefficiency)
+   crtc_bw = mult_frac(crtc_bw, hw_cfg->perf.ab_inefficiency, 100);
+   mdp5_cstate->new_crtc_bw = crtc_bw;
+
/*
 * we need a right hwmixer if the mode's width is greater than a single
 * LM's max width
@@ -785,6 +813,7 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc,
 {
struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc->state);
+   struct mdp5_kms *mdp5_kms = get_kms(crtc);
struct drm_device *dev = crtc->dev;
unsigned long flags;
 
@@ -808,6 +837,12 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc,
 
blend_setup(crtc);
 
+   if (mdp5_cstate->old_crtc_bw < mdp5_cstate->new_crtc_bw) {
+   DBG("UP BW to %lld\n", mdp5_cstate->new_crtc_bw);
+   mdp5_kms_set_bandwidth(mdp5_kms);
+   mdp5_cstate->old_crtc_bw = mdp5_cstate->new_crtc_bw;
+   }
+
/* PP_DONE irq is only used by command mode for now.
 * It is better to request pending before FLUSH and START trigger
 * to make sure no pp_done irq missed.
@@ -1155,6 +1190,7 @@ static void mdp5_crtc_vblank_irq(struct mdp_irq *irq, 
uint32_t irqstatus)
 {
struct mdp5_crtc *mdp5_crtc = container_of(irq, struct mdp5_crtc, 
vblank);
struct drm_crtc *crtc = _crtc->base;
+   struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc->state);
struct msm_drm_private *priv = crtc->dev->dev_private;
unsigned pending;
 
@@

[Freedreno] [PATCH 3/7] drm/msm/mdp5: use drm_plane_state for pixel blend mode

2021-05-25 Thread Dmitry Baryshkov

Use drm_plane_state's 'pixel_blend_mode' field rather than using
'premultiplied' field to mdp5_plane_state.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c  | 6 --
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h   | 1 -
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 5 +
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index b98d5abafd1f..ed4d91420417 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -301,7 +301,8 @@ static void blend_setup(struct drm_crtc *crtc)
 
DBG("Stage %d fg_alpha %x bg_alpha %x", i, fg_alpha, bg_alpha);
 
-   if (format->alpha_enable && pstates[i]->premultiplied) {
+   if (format->alpha_enable &&
+   pstates[i]->base.pixel_blend_mode == 
DRM_MODE_BLEND_PREMULTI) {
blend_op = MDP5_LM_BLEND_OP_MODE_FG_ALPHA(FG_CONST) |
MDP5_LM_BLEND_OP_MODE_BG_ALPHA(FG_PIXEL);
if (fg_alpha != 0xff) {
@@ -312,7 +313,8 @@ static void blend_setup(struct drm_crtc *crtc)
} else {
blend_op |= MDP5_LM_BLEND_OP_MODE_BG_INV_ALPHA;
}
-   } else if (format->alpha_enable) {
+   } else if (format->alpha_enable &&
+  pstates[i]->base.pixel_blend_mode == 
DRM_MODE_BLEND_COVERAGE) {
blend_op = MDP5_LM_BLEND_OP_MODE_FG_ALPHA(FG_PIXEL) |
MDP5_LM_BLEND_OP_MODE_BG_ALPHA(FG_PIXEL);
if (fg_alpha != 0xff) {
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
index d7e04e99fb4e..d124c9bcdc60 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
@@ -99,7 +99,6 @@ struct mdp5_plane_state {
struct mdp5_hw_pipe *r_hwpipe;  /* right hwpipe */
 
/* aligned with property */
-   uint8_t premultiplied;
uint8_t zpos;
 
/* assigned by crtc blender */
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index 0fd1d10352aa..90cd825df16b 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -166,7 +166,7 @@ mdp5_plane_atomic_print_state(struct drm_printer *p,
drm_printf(p, "\tright-hwpipe=%s\n",
   pstate->r_hwpipe ? pstate->r_hwpipe->name :
  "(null)");
-   drm_printf(p, "\tpremultiplied=%u\n", pstate->premultiplied);
+   drm_printf(p, "\tblend_mode=%u\n", pstate->base.pixel_blend_mode);
drm_printf(p, "\tzpos=%u\n", pstate->zpos);
drm_printf(p, "\talpha=%u\n", pstate->base.alpha);
drm_printf(p, "\tstage=%s\n", stage2name(pstate->stage));
@@ -182,9 +182,6 @@ static void mdp5_plane_reset(struct drm_plane *plane)
kfree(to_mdp5_plane_state(plane->state));
mdp5_state = kzalloc(sizeof(*mdp5_state), GFP_KERNEL);
 
-   /* assign default blend parameters */
-   mdp5_state->premultiplied = 0;
-
if (plane->type == DRM_PLANE_TYPE_PRIMARY)
mdp5_state->zpos = STAGE_BASE;
else
-- 
2.30.2

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 2/7] drm/msm/mdp5: use drm_plane_state for storing alpha value

2021-05-25 Thread Dmitry Baryshkov

Use drm_plane_state's 'alpha' field rather than adding extra 'alpha'
field to mdp5_plane_state.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c  | 4 ++--
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h   | 1 -
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 3 +--
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index f5d71b274079..b98d5abafd1f 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -291,8 +291,8 @@ static void blend_setup(struct drm_crtc *crtc)
plane = pstates[i]->base.plane;
blend_op = MDP5_LM_BLEND_OP_MODE_FG_ALPHA(FG_CONST) |
MDP5_LM_BLEND_OP_MODE_BG_ALPHA(BG_CONST);
-   fg_alpha = pstates[i]->alpha;
-   bg_alpha = 0xFF - pstates[i]->alpha;
+   fg_alpha = pstates[i]->base.alpha >> 8;
+   bg_alpha = 0xFF - fg_alpha;
 
if (!format->alpha_enable && bg_alpha_enabled)
mixer_op_mode = 0;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
index 128866742593..d7e04e99fb4e 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
@@ -101,7 +101,6 @@ struct mdp5_plane_state {
/* aligned with property */
uint8_t premultiplied;
uint8_t zpos;
-   uint8_t alpha;
 
/* assigned by crtc blender */
enum mdp_mixer_stage_id stage;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index 8c29026d770d..0fd1d10352aa 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -168,7 +168,7 @@ mdp5_plane_atomic_print_state(struct drm_printer *p,
  "(null)");
drm_printf(p, "\tpremultiplied=%u\n", pstate->premultiplied);
drm_printf(p, "\tzpos=%u\n", pstate->zpos);
-   drm_printf(p, "\talpha=%u\n", pstate->alpha);
+   drm_printf(p, "\talpha=%u\n", pstate->base.alpha);
drm_printf(p, "\tstage=%s\n", stage2name(pstate->stage));
 }
 
@@ -183,7 +183,6 @@ static void mdp5_plane_reset(struct drm_plane *plane)
mdp5_state = kzalloc(sizeof(*mdp5_state), GFP_KERNEL);
 
/* assign default blend parameters */
-   mdp5_state->alpha = 255;
mdp5_state->premultiplied = 0;
 
if (plane->type == DRM_PLANE_TYPE_PRIMARY)
-- 
2.30.2

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 1/7] drm/msm/mdp5: use drm atomic helpers to handle base drm plane state

2021-05-25 Thread Dmitry Baryshkov

Use generic helpers code to manage drm_plane_state part of mdp5_plane
state instead of manually coding all the details.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index 8c9f2f492178..8c29026d770d 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -176,8 +176,8 @@ static void mdp5_plane_reset(struct drm_plane *plane)
 {
struct mdp5_plane_state *mdp5_state;
 
-   if (plane->state && plane->state->fb)
-   drm_framebuffer_put(plane->state->fb);
+   if (plane->state)
+   __drm_atomic_helper_plane_destroy_state(plane->state);
 
kfree(to_mdp5_plane_state(plane->state));
mdp5_state = kzalloc(sizeof(*mdp5_state), GFP_KERNEL);
@@ -191,9 +191,7 @@ static void mdp5_plane_reset(struct drm_plane *plane)
else
mdp5_state->zpos = STAGE0 + drm_plane_index(plane);
 
-   mdp5_state->base.plane = plane;
-
-   plane->state = _state->base;
+   __drm_atomic_helper_plane_reset(plane, _state->base);
 }
 
 static struct drm_plane_state *
-- 
2.30.2

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 0/7] drm/msm/mdp5: add properties and bandwidth management

2021-05-25 Thread Dmitry Baryshkov

Update MDP5 display driver to support current implementation of
alpha/blend mode/zpos properties. On top of that port bandwidth
management from DPU display driver.

The following changes since commit 8dbde399044b0f5acf704ab5f8116bd8b1dfcf95:

  drm/msm/dp: handle irq_hpd with sink_count = 0 correctly (2021-05-24 16:08:33 
-0700)

are available in the Git repository at:

  https://git.linaro.org/people/dmitry.baryshkov/kernel.git mdp5-update

for you to fetch changes up to 501c3f8c40e139d97b17240e0a5492a12b6c722d:

  drm/msm/mdp5: provide dynamic bandwidth management (2021-05-25 16:10:17 +0300)


Dmitry Baryshkov (6):
  drm/msm/mdp5: use drm atomic helpers to handle base drm plane state
  drm/msm/mdp5: use drm_plane_state for storing alpha value
  drm/msm/mdp5: use drm_plane_state for pixel blend mode
  drm/msm/mdp5: add support for alpha/blend_mode properties
  drm/msm/mdp5: switch to standard zpos property
  drm/msm/mdp5: provide dynamic bandwidth management

James Willcox (1):
  drm/msm/mdp5: add perf blocks for holding fudge factors

 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c   |  35 ++
 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h   |   7 ++
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c  |  56 -
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c   | 119 +--
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h   |  17 ++-
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 177 ++---
 6 files changed, 249 insertions(+), 162 deletions(-)


___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [v4 1/4] drm/panel-simple: Add basic DPCD backlight support

2021-05-25 Thread kernel test robot

Hi Rajeev,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-tip/drm-tip]
[also build test ERROR on next-20210525]
[cannot apply to robh/for-next drm-intel/for-linux-next 
drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master 
drm/drm-next v5.13-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Rajeev-Nandan/drm-Support-basic-DPCD-backlight-in-panel-simple-and-add-a-new-panel-ATNA33XC20/20210525-153326
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: arm-randconfig-r025-20210525 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 
99155e913e9bad5f7f8a247f8bb3a3ff3da74af1)
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# 
https://github.com/0day-ci/linux/commit/24e7ccb98951b0b4c7ae8a367273f8e73c074804
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Rajeev-Nandan/drm-Support-basic-DPCD-backlight-in-panel-simple-and-add-a-new-panel-ATNA33XC20/20210525-153326
git checkout 24e7ccb98951b0b4c7ae8a367273f8e73c074804
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/panel/panel-simple.c:185:32: error: field has incomplete 
>> type 'struct drm_edp_backlight_info'
   struct drm_edp_backlight_info info;
 ^
   drivers/gpu/drm/panel/panel-simple.c:185:9: note: forward declaration of 
'struct drm_edp_backlight_info'
   struct drm_edp_backlight_info info;
  ^
>> drivers/gpu/drm/panel/panel-simple.c:352:3: error: implicit declaration of 
>> function 'drm_edp_backlight_disable' 
>> [-Werror,-Wimplicit-function-declaration]
   drm_edp_backlight_disable(p->aux, >info);
   ^
   drivers/gpu/drm/panel/panel-simple.c:352:3: note: did you mean 
'backlight_disable'?
   include/linux/backlight.h:379:19: note: 'backlight_disable' declared here
   static inline int backlight_disable(struct backlight_device *bd)
 ^
>> drivers/gpu/drm/panel/panel-simple.c:352:32: error: no member named 'aux' in 
>> 'struct panel_simple'
   drm_edp_backlight_disable(p->aux, >info);
 ~  ^
>> drivers/gpu/drm/panel/panel-simple.c:527:3: error: implicit declaration of 
>> function 'drm_edp_backlight_enable' [-Werror,-Wimplicit-function-declaration]
   drm_edp_backlight_enable(p->aux, >info,
   ^
   drivers/gpu/drm/panel/panel-simple.c:527:3: note: did you mean 
'backlight_enable'?
   include/linux/backlight.h:363:19: note: 'backlight_enable' declared here
   static inline int backlight_enable(struct backlight_device *bd)
 ^
   drivers/gpu/drm/panel/panel-simple.c:527:31: error: no member named 'aux' in 
'struct panel_simple'
   drm_edp_backlight_enable(p->aux, >info,
~  ^
>> drivers/gpu/drm/panel/panel-simple.c:598:9: error: implicit declaration of 
>> function 'drm_edp_backlight_set_level' 
>> [-Werror,-Wimplicit-function-declaration]
   return drm_edp_backlight_set_level(p->aux, >info, 
bd->props.brightness);
  ^
   drivers/gpu/drm/panel/panel-simple.c:598:40: error: no member named 'aux' in 
'struct panel_simple'
   return drm_edp_backlight_set_level(p->aux, >info, 
bd->props.brightness);
  ~  ^
>> drivers/gpu/drm/panel/panel-simple.c:611:14: error: use of undeclared 
>> identifier 'EDP_DISPLAY_CTL_CAP_SIZE'
   u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
   ^
>> drivers/gpu/drm/panel/panel-simple.c:618:8: error: implicit declaration of 
>> function 'drm_dp_dpcd_read' [-Werror,-Wimplicit-function-declaration]
   ret = drm_dp_dpcd_read(panel->aux, DP_EDP_DPCD_REV, edp_dpcd,
 ^
   drivers/gpu/drm/panel/panel-simple.c:618:32: error: no member named 'aux' in 
'struct panel_simple'
   ret = drm_dp_dpcd_read(panel->aux, DP_EDP_DPCD_REV, edp_dpcd,
  ~  ^
>> drivers/gpu/drm/pa

[Freedreno] [PATCH] drm/msm/disp/dpu1/dpu_encoder: Drop unnecessary NULL checks after container_of

2021-05-25 Thread Guenter Roeck

The result of container_of() operations is never NULL unless the embedded
element is the first element of the structure. This is not the case here.
The NULL checks on the result of container_of() are therefore unnecessary
and misleading. Remove them.

This change was made automatically with the following Coccinelle script.

@@
type t;
identifier v;
statement s;
@@

<+...
(
  t v = container_of(...);
|
  v = container_of(...);
)
  ...
  when != v
- if (\( !v \| v == NULL \) ) s
...+>

Signed-off-by: Guenter Roeck 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 8d942052db8a..a573fe211375 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1453,11 +1453,6 @@ static void dpu_encoder_off_work(struct work_struct 
*work)
struct dpu_encoder_virt *dpu_enc = container_of(work,
struct dpu_encoder_virt, delayed_off_work.work);
 
-   if (!dpu_enc) {
-   DPU_ERROR("invalid dpu encoder\n");
-   return;
-   }
-
dpu_encoder_resource_control(_enc->base,
DPU_ENC_RC_EVENT_ENTER_IDLE);
 
@@ -1797,11 +1792,6 @@ static void dpu_encoder_vsync_event_work_handler(struct 
kthread_work *work)
struct dpu_encoder_virt, vsync_event_work);
ktime_t wakeup_time;
 
-   if (!dpu_enc) {
-   DPU_ERROR("invalid dpu encoder\n");
-   return;
-   }
-
if (dpu_encoder_vsync_time(_enc->base, _time))
return;
 
-- 
2.25.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [v3 1/2] dt-bindings: backlight: add DisplayPort aux backlight

2021-05-25 Thread rajeevny


Hi,

On 18-05-2021 01:51, Doug Anderson wrote:

Hi,

On Tue, May 11, 2021 at 4:17 PM Doug Anderson  
wrote:


Hi,

On Tue, May 11, 2021 at 11:12 AM  wrote:
>
> On 01-05-2021 03:08, Doug Anderson wrote:
> > Hi,
> >
> > On Fri, Apr 30, 2021 at 8:10 AM  wrote:
> >>
> >> On 30-04-2021 02:33, Doug Anderson wrote:
> >> > Hi,
> >> >
> >> > On Thu, Apr 29, 2021 at 11:04 AM Rob Herring  wrote:
> >> >>
> >> >> On Mon, Apr 26, 2021 at 11:29:15AM +0530, Rajeev Nandan wrote:
> >> >> > Add bindings for DisplayPort aux backlight driver.
> >> >> >
> >> >> > Changes in v2:
> >> >> > - New
> >> >> >
> >> >> > Signed-off-by: Rajeev Nandan 
> >> >> > ---
> >> >> >  .../bindings/leds/backlight/dp-aux-backlight.yaml  | 49 
++
> >> >> >  1 file changed, 49 insertions(+)
> >> >> >  create mode 100644 
Documentation/devicetree/bindings/leds/backlight/dp-aux-backlight.yaml
> >> >> >
> >> >> > diff --git 
a/Documentation/devicetree/bindings/leds/backlight/dp-aux-backlight.yaml 
b/Documentation/devicetree/bindings/leds/backlight/dp-aux-backlight.yaml
> >> >> > new file mode 100644
> >> >> > index ..0fa8bf0
> >> >> > --- /dev/null
> >> >> > +++ 
b/Documentation/devicetree/bindings/leds/backlight/dp-aux-backlight.yaml
> >> >> > @@ -0,0 +1,49 @@
> >> >> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> >> > +%YAML 1.2
> >> >> > +---
> >> >> > +$id: 
http://devicetree.org/schemas/leds/backlight/dp-aux-backlight.yaml#
> >> >> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> >> > +
> >> >> > +title: DisplayPort aux backlight driver bindings
> >> >> > +
> >> >> > +maintainers:
> >> >> > +  - Rajeev Nandan 
> >> >> > +
> >> >> > +description:
> >> >> > +  Backlight driver to control the brightness over DisplayPort aux 
channel.
> >> >> > +
> >> >> > +allOf:
> >> >> > +  - $ref: common.yaml#
> >> >> > +
> >> >> > +properties:
> >> >> > +  compatible:
> >> >> > +const: dp-aux-backlight
> >> >> > +
> >> >> > +  ddc-i2c-bus:
> >> >> > +$ref: /schemas/types.yaml#/definitions/phandle
> >> >> > +description:
> >> >> > +  A phandle to the system I2C controller connected to the DDC 
bus used
> >> >> > +  for the DisplayPort AUX channel.
> >> >> > +
> >> >> > +  enable-gpios:
> >> >> > +maxItems: 1
> >> >> > +description: GPIO specifier for backlight enable pin.
> >> >> > +
> >> >> > +  max-brightness: true
> >> >> > +
> >> >> > +required:
> >> >> > +  - compatible
> >> >> > +  - ddc-i2c-bus
> >> >> > +
> >> >> > +additionalProperties: false
> >> >> > +
> >> >> > +examples:
> >> >> > +  - |
> >> >> > +backlight {
> >> >> > +compatible = "dp-aux-backlight";
> >> >> > +ddc-i2c-bus = <_bridge>;
> >> >> > +enable-gpios = < 12 GPIO_ACTIVE_HIGH>;
> >> >>
> >> >> So the DDC bus is connected to a backlight and also a panel? This
> >> >> binding is not reflecting the h/w, but rather what you want for some
> >> >> driver.
> >> >>
> >> >> There's only one thing here and that's an eDP panel which supports
> >> >> backlight control via DP aux channel. You can figure all that out from
> >> >> the panel's compatible and/or reading the EDID.
> >> >>
> >> >> You might also be interested in this thread:
> >> >>
> >> >> https://lore.kernel.org/lkml/yiksdtjcihgnv...@orome.fritz.box/
> >> >
> >> > I think Rajeev needs to rework everything anyway as per:
> >> >
> >> > https://lore.kernel.org/r/87zgxl5qar@intel.com
> >> >
> >> > ...but you're right that it makes sense not to model the backlight as
> >> > a separate node in the device tree. The panel driver can handle
> >> > setting up the backlight.
> >> >
> >> > -Doug
> >>
> >> It was not a good idea to create a separate backlight driver and use
> >> ddc-i2c-bus to get access to DP aux. I am working to move the code
> >> to the panel driver and to utilize the new DRM helper functions
> >> (drm_edp_backlight_*) Lyude has added [1].
> >>
> >> To use these helper functions, the panel driver should have access to
> >> the
> >> "struct drm_dp_aux *". The simple-panel has a "ddc-i2c-bus" property
> >> to give the panel access to the DDC bus and is currently being used to
> >> get the EDID from the panel. Can I use the same ddc bus i2c_adapter to
> >> get
> >> the "struct drm_dp_aux *"?
> >>
> >> As per the suggestion [2], I get the "struct drm_dp_aux *" from the
> >> i2c_adapter of ddc bus (maybe I didn't understand the suggestion
> >> correctly),
> >> and, it turned out, the way I have implemented is not the right way
> >> [3].
> >> So, I am afraid to use the same method in the panel driver.
> >>
> >>
> >> [1] https://lore.kernel.org/dri-devel/871rb5bcf9@intel.com/
> >> [2] https://www.spinics.net/lists/dri-devel/msg295429.html
> >> [3]
> >> 
https://lore.kernel.org/dri-devel/2021042616.4lc3ekxjugjr3...@maple.lan/
> >
> > So it's definitely up to maintainers, not me. ...but I guess I would
> > have expected something like a new property called "ddc-aux-bus". Then
> > you'd have to create a new API call

[Freedreno] [v4 2/4] drm/panel-simple: Support for delays between GPIO & regulator

2021-05-25 Thread Rajeev Nandan

Some panels datasheets may specify a delay between the enable GPIO and
the regulator. Support this in panel-simple.

Signed-off-by: Rajeev Nandan 
---

Changes in v4:
- New

 drivers/gpu/drm/panel/panel-simple.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index f9e4e60..caed71b 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -134,6 +134,22 @@ struct panel_desc {
unsigned int prepare_to_enable;
 
/**
+* @delay.power_to_enable: Time for the power to enable the 
display on.
+*
+* The time (in milliseconds) that it takes for the panel to
+* turn the display on.
+*/
+   unsigned int power_to_enable;
+
+   /**
+* @delay.disable_to_power_off: Time for the disable to power 
the display off.
+*
+* The time (in milliseconds) that it takes for the panel to
+* turn the display off.
+*/
+   unsigned int disable_to_power_off;
+
+   /**
 * @delay.enable: Time for the panel to display a valid frame.
 *
 * The time (in milliseconds) that it takes for the panel to
@@ -367,6 +383,10 @@ static int panel_simple_suspend(struct device *dev)
struct panel_simple *p = dev_get_drvdata(dev);
 
gpiod_set_value_cansleep(p->enable_gpio, 0);
+
+   if (p->desc->delay.disable_to_power_off)
+   msleep(p->desc->delay.disable_to_power_off);
+
regulator_disable(p->supply);
p->unprepared_time = ktime_get();
 
@@ -427,6 +447,9 @@ static int panel_simple_prepare_once(struct panel_simple *p)
return err;
}
 
+   if (p->desc->delay.power_to_enable)
+   msleep(p->desc->delay.power_to_enable);
+
gpiod_set_value_cansleep(p->enable_gpio, 1);
 
delay = p->desc->delay.prepare;
-- 
2.7.4

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [v4 1/4] drm/panel-simple: Add basic DPCD backlight support

2021-05-25 Thread Rajeev Nandan

Add basic support of panel backlight control over eDP aux channel
using VESA's standard backlight control interface.

Signed-off-by: Rajeev Nandan 
---

This patch depends on [1] (drm/panel: panel-simple: Stash DP AUX bus; 
allow using it for DDC) 

Changes in v4:
- New

[1] 
https://lore.kernel.org/dri-devel/20210524165920.v8.7.I18e60221f6d048d14d6c50a770b15f356fa75092@changeid/

 drivers/gpu/drm/panel/panel-simple.c | 99 ++--
 1 file changed, 96 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index b09be6e..f9e4e60 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -21,6 +21,7 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -171,6 +172,19 @@ struct panel_desc {
 
/** @connector_type: LVDS, eDP, DSI, DPI, etc. */
int connector_type;
+
+   /**
+* @uses_dpcd_backlight: Panel supports eDP dpcd backlight control.
+*
+* Set true, if the panel supports backlight control over eDP AUX 
channel
+* using DPCD registers as per VESA's standard.
+*/
+   bool uses_dpcd_backlight;
+};
+
+struct edp_backlight {
+   struct backlight_device *dev;
+   struct drm_edp_backlight_info info;
 };
 
 struct panel_simple {
@@ -194,6 +208,8 @@ struct panel_simple {
 
struct edid *edid;
 
+   struct edp_backlight *edp_bl;
+
struct drm_display_mode override_mode;
 
enum drm_panel_orientation orientation;
@@ -330,10 +346,14 @@ static void panel_simple_wait(ktime_t start_ktime, 
unsigned int min_ms)
 static int panel_simple_disable(struct drm_panel *panel)
 {
struct panel_simple *p = to_panel_simple(panel);
+   struct edp_backlight *bl = p->edp_bl;
 
if (!p->enabled)
return 0;
 
+   if (p->desc->uses_dpcd_backlight && bl)
+   drm_edp_backlight_disable(p->aux, >info);
+
if (p->desc->delay.disable)
msleep(p->desc->delay.disable);
 
@@ -496,6 +516,7 @@ static int panel_simple_prepare(struct drm_panel *panel)
 static int panel_simple_enable(struct drm_panel *panel)
 {
struct panel_simple *p = to_panel_simple(panel);
+   struct edp_backlight *bl = p->edp_bl;
 
if (p->enabled)
return 0;
@@ -505,6 +526,10 @@ static int panel_simple_enable(struct drm_panel *panel)
 
panel_simple_wait(p->prepared_time, p->desc->delay.prepare_to_enable);
 
+   if (p->desc->uses_dpcd_backlight && bl)
+   drm_edp_backlight_enable(p->aux, >info,
+bl->dev->props.brightness);
+
p->enabled = true;
 
return 0;
@@ -565,6 +590,59 @@ static const struct drm_panel_funcs panel_simple_funcs = {
.get_timings = panel_simple_get_timings,
 };
 
+static int edp_backlight_update_status(struct backlight_device *bd)
+{
+   struct panel_simple *p = bl_get_data(bd);
+   struct edp_backlight *bl = p->edp_bl;
+
+   if (!p->enabled)
+   return 0;
+
+   return drm_edp_backlight_set_level(p->aux, >info, 
bd->props.brightness);
+}
+
+static const struct backlight_ops edp_backlight_ops = {
+   .update_status = edp_backlight_update_status,
+};
+
+static int edp_backlight_register(struct device *dev, struct panel_simple 
*panel)
+{
+   struct edp_backlight *bl;
+   struct backlight_properties props = { 0 };
+   u16 current_level;
+   u8 current_mode;
+   u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
+   int ret;
+
+   bl = devm_kzalloc(dev, sizeof(*bl), GFP_KERNEL);
+   if (!bl)
+   return -ENOMEM;
+
+   ret = drm_dp_dpcd_read(panel->aux, DP_EDP_DPCD_REV, edp_dpcd,
+  EDP_DISPLAY_CTL_CAP_SIZE);
+   if (ret < 0)
+   return ret;
+
+   ret = drm_edp_backlight_init(panel->aux, >info, 0, edp_dpcd,
+_level, _mode);
+   if (ret < 0)
+   return ret;
+
+   props.type = BACKLIGHT_RAW;
+   props.brightness = current_level;
+   props.max_brightness = bl->info.max;
+
+   bl->dev = devm_backlight_device_register(dev, "edp_backlight",
+   dev, panel,
+   _backlight_ops, );
+   if (IS_ERR(bl->dev))
+   return PTR_ERR(bl->dev);
+
+   panel->edp_bl = bl;
+
+   return 0;
+}
+
 static struct panel_desc panel_dpi;
 
 static int panel_dpi_probe(struct device *dev,
@@ -796,9 +874,24 @@ static int panel_simple_probe(struct device *dev, const 
struct panel_desc *desc,
 
drm_panel_init(>base, dev, _simple_funcs, connector_type);
 
-   err = drm_panel_of_backlight(>base);
-   if (err)
-   goto disable_pm_runtime;
+   if (panel->desc->uses_dpcd_backlight) {
+   if (!panel->aux) {
+

[Freedreno] [v4 4/4] drm/panel-simple: Add Samsung ATNA33XC20

2021-05-25 Thread Rajeev Nandan

Add Samsung 13.3" FHD eDP AMOLED panel.

Signed-off-by: Rajeev Nandan 
---

Changes in v4:
- New

 drivers/gpu/drm/panel/panel-simple.c | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index caed71b..21af794 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -3644,6 +3644,37 @@ static const struct panel_desc rocktech_rk101ii01d_ct = {
.connector_type = DRM_MODE_CONNECTOR_LVDS,
 };
 
+static const struct drm_display_mode samsung_atna33xc20_mode = {
+   .clock = 138770,
+   .hdisplay = 1920,
+   .hsync_start = 1920 + 48,
+   .hsync_end = 1920 + 48 + 32,
+   .htotal = 1920 + 48 + 32 + 80,
+   .vdisplay = 1080,
+   .vsync_start = 1080 + 8,
+   .vsync_end = 1080 + 8 + 8,
+   .vtotal = 1080 + 8 + 8 + 16,
+   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+static const struct panel_desc samsung_atna33xc20 = {
+   .modes = _atna33xc20_mode,
+   .num_modes = 1,
+   .bpc = 10,
+   .size = {
+   .width = 294,
+   .height = 165,
+   },
+   .delay = {
+   .disable_to_power_off = 150,
+   .power_to_enable = 150,
+   .hpd_absent_delay = 200,
+   .unprepare = 500,
+   },
+   .connector_type = DRM_MODE_CONNECTOR_eDP,
+   .uses_dpcd_backlight = true,
+};
+
 static const struct drm_display_mode samsung_lsn122dl01_c01_mode = {
.clock = 271560,
.hdisplay = 2560,
@@ -4645,6 +4676,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "rocktech,rk101ii01d-ct",
.data = _rk101ii01d_ct,
}, {
+   .compatible = "samsung,atna33xc20",
+   .data = _atna33xc20,
+   }, {
.compatible = "samsung,lsn122dl01-c01",
.data = _lsn122dl01_c01,
}, {
-- 
2.7.4

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [v4 3/4] dt-bindings: display: simple: Add Samsung ATNA33XC20

2021-05-25 Thread Rajeev Nandan

Add Samsung 13.3" FHD eDP AMOLED panel.

Signed-off-by: Rajeev Nandan 
---

Changes in v4:
- New

 Documentation/devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml 
b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 4a0a5e1..f5acfd6 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -242,6 +242,8 @@ properties:
   - rocktech,rk101ii01d-ct
 # Rocktech Display Ltd. RK070ER9427 800(RGB)x480 TFT LCD panel
   - rocktech,rk070er9427
+# Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel
+  - samsung,atna33xc20
 # Samsung 12.2" (2560x1600 pixels) TFT LCD panel
   - samsung,lsn122dl01-c01
 # Samsung Electronics 10.1" WSVGA TFT LCD panel
-- 
2.7.4

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [v4 0/4] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20

2021-05-25 Thread Rajeev Nandan

This series adds the support for the eDP panel that needs the backlight
controlling over the DP AUX channel using DPCD registers of the panel
as per the VESA's standard.

This series also adds support for the Samsung eDP AMOLED panel that
needs DP AUX to control the backlight, and introduces new delays in the
@panel_desc.delay to support this panel.

This patch series depends on the following two series:
- Doug's series [1], exposed the DP AUX channel to the panel-simple.
- Lyude's series [2], introduced new drm helper functions for DPCD
backlight.

This series is the logical successor to the series [3].

Changes in v1:
- Created dpcd backlight helper with very basic functionality, added
backlight registration in the ti-sn65dsi86 bridge driver.

Changes in v2:
- Created a new DisplayPort aux backlight driver and moved the code from
drm_dp_aux_backlight.c (v1) to the new driver.

Changes in v3:
- Fixed module compilation (kernel test bot).

Changes in v4:
- Added basic DPCD backlight support in panel-simple.
- Added support for a new Samsung panel ATNA33XC20 that needs DPCD
backlight controlling and has a requirement of delays between enable
GPIO and regulator.

[1]
https://lore.kernel.org/dri-devel/20210525000159.3384921-1-diand...@chromium.org/
[2] https://lore.kernel.org/dri-devel/20210514181504.565252-1-ly...@redhat.com/
[3]
https://lore.kernel.org/dri-devel/1619416756-3533-1-git-send-email-rajee...@codeaurora.org/

Rajeev Nandan (4):
drm/panel-simple: Add basic DPCD backlight support
drm/panel-simple: Support for delays between GPIO & regulator
dt-bindings: display: simple: Add Samsung ATNA33XC20
drm/panel-simple: Add Samsung ATNA33XC20

.../bindings/display/panel/panel-simple.yaml | 2 +
drivers/gpu/drm/panel/panel-simple.c | 156 -
2 files changed, 155 insertions(+), 3 deletions(-)

--
2.7.4

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [RFC PATCH 00/13] drm/msm: Add Display Stream Compression Support

Re: [Freedreno] [RFC PATCH 02/13] dt-bindings: msm/dsi: Document Display Stream Compression (DSC) parameters

Re: [Freedreno] [PATCH v2 4/6] drm/msm/dpu: replace IRQ lookup with the data in hw catalog

Re: [Freedreno] [PATCH] drm/msm/disp/dpu1/dpu_encoder: Drop unnecessary NULL checks after container_of

Re: [Freedreno] [PATCH] drm/msm: remove unneeded variable ret

Re: [Freedreno] [PATCH] drm/msm/dp: remove the repeated declaration

Re: [Freedreno] [PATCH] drm/msm/dp: Drop unnecessary NULL checks after container_of

Re: [Freedreno] [v4 1/4] drm/panel-simple: Add basic DPCD backlight support

Re: [Freedreno] [v4 3/4] dt-bindings: display: simple: Add Samsung ATNA33XC20

Re: [Freedreno] [v4 4/4] drm/panel-simple: Add Samsung ATNA33XC20

Re: [Freedreno] [v4 2/4] drm/panel-simple: Support for delays between GPIO & regulator

Re: [Freedreno] [PATCH 7/7] drm/msm/mdp5: provide dynamic bandwidth management

Re: [Freedreno] [PATCH 1/2] iommu/arm-smmu-qcom: Skip the TTBR1 quirk for db820c.

Re: [Freedreno] [PATCH] drm/msm/disp/dpu1/dpu_encoder: Drop unnecessary NULL checks after container_of

Re: [Freedreno] [PATCH] drm/msm/dp: remove the repeated declaration

[Freedreno] [PATCH] drm/msm/dp: remove the repeated declaration

[Freedreno] [PATCH 5/7] drm/msm/mdp5: switch to standard zpos property

[Freedreno] [PATCH 4/7] drm/msm/mdp5: add support for alpha/blend_mode properties

[Freedreno] [PATCH 6/7] drm/msm/mdp5: add perf blocks for holding fudge factors

[Freedreno] [PATCH 7/7] drm/msm/mdp5: provide dynamic bandwidth management

[Freedreno] [PATCH 3/7] drm/msm/mdp5: use drm_plane_state for pixel blend mode

[Freedreno] [PATCH 2/7] drm/msm/mdp5: use drm_plane_state for storing alpha value

[Freedreno] [PATCH 1/7] drm/msm/mdp5: use drm atomic helpers to handle base drm plane state

[Freedreno] [PATCH 0/7] drm/msm/mdp5: add properties and bandwidth management

Re: [Freedreno] [v4 1/4] drm/panel-simple: Add basic DPCD backlight support

[Freedreno] [PATCH] drm/msm/disp/dpu1/dpu_encoder: Drop unnecessary NULL checks after container_of

Re: [Freedreno] [v3 1/2] dt-bindings: backlight: add DisplayPort aux backlight

[Freedreno] [v4 2/4] drm/panel-simple: Support for delays between GPIO & regulator

[Freedreno] [v4 1/4] drm/panel-simple: Add basic DPCD backlight support

[Freedreno] [v4 4/4] drm/panel-simple: Add Samsung ATNA33XC20

[Freedreno] [v4 3/4] dt-bindings: display: simple: Add Samsung ATNA33XC20

[Freedreno] [v4 0/4] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20

32 matches

Site Navigation

Mail list logo

Footer information