Re: [PATCH v2 4/7] drm/msm/dpu: dont use encoder->crtc in atomic path

2019-03-07 Thread Sean Paul 
On Wed, Mar 06, 2019 at 06:03:05PM -0800, Jeykumar Sankaran wrote:
> On 2019-03-04 10:09, Sean Paul wrote:
> > On Wed, Feb 13, 2019 at 05:19:13PM -0800, Jeykumar Sankaran wrote:
> > > encoder->crtc is not really meaningful for atomic path. Use
> > > crtc->encoder_mask to identify the crtc attached with
> > > an encoder.
> > > 
> > > Signed-off-by: Jeykumar Sankaran 
> > > ---
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > index 45617b9..0a19124 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > @@ -961,6 +961,7 @@ static void dpu_encoder_virt_mode_set(struct
> > drm_encoder *drm_enc,
> > >   struct dpu_kms *dpu_kms;
> > >   struct list_head *connector_list;
> > >   struct drm_connector *conn = NULL, *conn_iter;
> > > + struct drm_crtc *drm_crtc;
> > >   struct dpu_rm_hw_iter pp_iter, ctl_iter;
> > >   struct msm_display_topology topology;
> > >   struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
> > > @@ -992,10 +993,14 @@ static void dpu_encoder_virt_mode_set(struct
> > drm_encoder *drm_enc,
> > >   return;
> > >   }
> > > 
> > > + drm_for_each_crtc(drm_crtc, drm_enc->dev)
> > > + if (drm_crtc->state->encoder_mask &
> > drm_encoder_mask(drm_enc))
> > > + break;
> > 
> > You should check whether you actually found the crtc, or are just using
> > the last
> > one in the list.
> > 
> I see you have pulled in the this entire series on msm-next. If it is the
> right
> thing to do, I can address this comment in a separate patch on top instead
> of
> posting v2.

Yes, please do.

Thanks,

Sean

> 
> Thanks
> Jeykumar S.
> > Sean
> > 
> > > +
> > >   topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
> > > 
> > >   /* Reserve dynamic resources now. Indicating non-AtomicTest phase
> > */
> > > - ret = dpu_rm_reserve(_kms->rm, drm_enc, drm_enc->crtc->state,
> > > + ret = dpu_rm_reserve(_kms->rm, drm_enc, drm_crtc->state,
> > >topology, false);
> > >   if (ret) {
> > >   DPU_ERROR_ENC(dpu_enc,
> > > --
> > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> > Forum,
> > > a Linux Foundation Collaborative Project
> > > 
> 
> -- 
> Jeykumar S

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/7] drm/msm/dpu: dont use encoder->crtc in atomic path

2019-03-06 Thread Jeykumar Sankaran 

On 2019-03-04 10:09, Sean Paul wrote:

On Wed, Feb 13, 2019 at 05:19:13PM -0800, Jeykumar Sankaran wrote:

encoder->crtc is not really meaningful for atomic path. Use
crtc->encoder_mask to identify the crtc attached with
an encoder.

Signed-off-by: Jeykumar Sankaran 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c

b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c

index 45617b9..0a19124 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -961,6 +961,7 @@ static void dpu_encoder_virt_mode_set(struct

drm_encoder *drm_enc,

struct dpu_kms *dpu_kms;
struct list_head *connector_list;
struct drm_connector *conn = NULL, *conn_iter;
+   struct drm_crtc *drm_crtc;
struct dpu_rm_hw_iter pp_iter, ctl_iter;
struct msm_display_topology topology;
struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
@@ -992,10 +993,14 @@ static void dpu_encoder_virt_mode_set(struct

drm_encoder *drm_enc,

return;
}

+   drm_for_each_crtc(drm_crtc, drm_enc->dev)
+   if (drm_crtc->state->encoder_mask &

drm_encoder_mask(drm_enc))

+   break;


You should check whether you actually found the crtc, or are just using
the last
one in the list.

I see you have pulled in the this entire series on msm-next. If it is 
the right
thing to do, I can address this comment in a separate patch on top 
instead of

posting v2.

Thanks
Jeykumar S.

Sean


+
topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);

/* Reserve dynamic resources now. Indicating non-AtomicTest phase

*/

-   ret = dpu_rm_reserve(_kms->rm, drm_enc, drm_enc->crtc->state,
+   ret = dpu_rm_reserve(_kms->rm, drm_enc, drm_crtc->state,
 topology, false);
if (ret) {
DPU_ERROR_ENC(dpu_enc,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora

Forum,

a Linux Foundation Collaborative Project



--
Jeykumar S
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/7] drm/msm/dpu: dont use encoder->crtc in atomic path

2019-03-04 Thread Sean Paul 
On Wed, Feb 13, 2019 at 05:19:13PM -0800, Jeykumar Sankaran wrote:
> encoder->crtc is not really meaningful for atomic path. Use
> crtc->encoder_mask to identify the crtc attached with
> an encoder.
> 
> Signed-off-by: Jeykumar Sankaran 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 45617b9..0a19124 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -961,6 +961,7 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder 
> *drm_enc,
>   struct dpu_kms *dpu_kms;
>   struct list_head *connector_list;
>   struct drm_connector *conn = NULL, *conn_iter;
> + struct drm_crtc *drm_crtc;
>   struct dpu_rm_hw_iter pp_iter, ctl_iter;
>   struct msm_display_topology topology;
>   struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
> @@ -992,10 +993,14 @@ static void dpu_encoder_virt_mode_set(struct 
> drm_encoder *drm_enc,
>   return;
>   }
>  
> + drm_for_each_crtc(drm_crtc, drm_enc->dev)
> + if (drm_crtc->state->encoder_mask & drm_encoder_mask(drm_enc))
> + break;

You should check whether you actually found the crtc, or are just using the last
one in the list.

Sean

> +
>   topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
>  
>   /* Reserve dynamic resources now. Indicating non-AtomicTest phase */
> - ret = dpu_rm_reserve(_kms->rm, drm_enc, drm_enc->crtc->state,
> + ret = dpu_rm_reserve(_kms->rm, drm_enc, drm_crtc->state,
>topology, false);
>   if (ret) {
>   DPU_ERROR_ENC(dpu_enc,
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 4/7] drm/msm/dpu: dont use encoder->crtc in atomic path

2019-02-13 Thread Jeykumar Sankaran 
encoder->crtc is not really meaningful for atomic path. Use
crtc->encoder_mask to identify the crtc attached with
an encoder.

Signed-off-by: Jeykumar Sankaran 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 45617b9..0a19124 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -961,6 +961,7 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder 
*drm_enc,
struct dpu_kms *dpu_kms;
struct list_head *connector_list;
struct drm_connector *conn = NULL, *conn_iter;
+   struct drm_crtc *drm_crtc;
struct dpu_rm_hw_iter pp_iter, ctl_iter;
struct msm_display_topology topology;
struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
@@ -992,10 +993,14 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder 
*drm_enc,
return;
}
 
+   drm_for_each_crtc(drm_crtc, drm_enc->dev)
+   if (drm_crtc->state->encoder_mask & drm_encoder_mask(drm_enc))
+   break;
+
topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
 
/* Reserve dynamic resources now. Indicating non-AtomicTest phase */
-   ret = dpu_rm_reserve(_kms->rm, drm_enc, drm_enc->crtc->state,
+   ret = dpu_rm_reserve(_kms->rm, drm_enc, drm_crtc->state,
 topology, false);
if (ret) {
DPU_ERROR_ENC(dpu_enc,
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel