Re: [PATCH v2 13/19] drm/msm/dp: add VSC SDP support for YUV420 over DP

2024-02-11 Thread Dmitry Baryshkov
On Sun, 11 Feb 2024 at 19:18, Abhinav Kumar  wrote:
>
>
>
> On 2/10/2024 10:57 PM, Dmitry Baryshkov wrote:
> > On Sun, 11 Feb 2024 at 06:06, Abhinav Kumar  
> > wrote:
> >>
> >>
> >>
> >> On 2/10/2024 1:46 PM, Dmitry Baryshkov wrote:
> >>> On Sat, 10 Feb 2024 at 20:50, Abhinav Kumar  
> >>> wrote:
> 
> 
> 
>  On 2/10/2024 10:14 AM, Abhinav Kumar wrote:
> >
> >
> > On 2/10/2024 2:09 AM, Dmitry Baryshkov wrote:
> >> On Sat, 10 Feb 2024 at 03:52, Paloma Arellano
> >>  wrote:
> >>>
> >>> Add support to pack and send the VSC SDP packet for DP. This therefore
> >>> allows the transmision of format information to the sinks which is
> >>> needed for YUV420 support over DP.
> >>>
> >>> Changes in v2:
> >>>- Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
> >>>- Remove dp_sdp from the dp_catalog struct since this data 
> >>> is
> >>>  being allocated at the point used
> >>>- Create a new function in dp_utils to pack the VSC SDP 
> >>> data
> >>>  into a buffer
> >>>- Create a new function that packs the SDP header bytes 
> >>> into a
> >>>  buffer. This function is made generic so that it can be
> >>>  utilized by dp_audio
> >>>  header bytes into a buffer
> >>>- Create a new function in dp_utils that takes the packed
> >>> buffer
> >>>  and writes to the DP_GENERIC0_* registers
> >>>- Split the dp_catalog_panel_config_vsc_sdp() function 
> >>> into two
> >>>  to disable/enable sending VSC SDP packets
> >>>- Check the DP HW version using the original useage of
> >>>  dp_catalog_hw_revision() and correct the version checking
> >>>  logic
> >>>- Rename dp_panel_setup_vsc_sdp() to
> >>>  dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
> >>>  currently VSC SDP is only being set up to support YUV420
> >>> modes
> >>>
> >>> Signed-off-by: Paloma Arellano 
> >>> ---
> >>> drivers/gpu/drm/msm/dp/dp_catalog.c | 105 
> >>> 
> >>> drivers/gpu/drm/msm/dp/dp_catalog.h |   6 ++
> >>> drivers/gpu/drm/msm/dp/dp_ctrl.c|   4 ++
> >>> drivers/gpu/drm/msm/dp/dp_panel.c   |  59 
> >>> drivers/gpu/drm/msm/dp/dp_reg.h |   3 +
> >>> drivers/gpu/drm/msm/dp/dp_utils.c   |  80 +
> >>> drivers/gpu/drm/msm/dp/dp_utils.h   |   3 +
> >>> 7 files changed, 260 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
> >>> b/drivers/gpu/drm/msm/dp/dp_catalog.c
> >>> index 5d84c089e520a..0f28a4897b7b7 100644
> >>> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> >>> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> >>> @@ -901,6 +901,111 @@ int dp_catalog_panel_timing_cfg(struct
> >>> dp_catalog *dp_catalog)
> >>>return 0;
> >>> }
> >>>
> >>
> >> 
> >>
> >>> +static int dp_panel_setup_vsc_sdp_yuv_420(struct dp_panel *dp_panel)
> >>> +{
> >>> +   struct dp_catalog *catalog;
> >>> +   struct dp_panel_private *panel;
> >>> +   struct dp_display_mode *dp_mode;
> >>> +   struct dp_sdp_header sdp_header;
> >>> +   struct drm_dp_vsc_sdp vsc_sdp_data;
> >>> +   size_t buff_size;
> >>> +   u32 gen_buffer[10];
> >>> +   int rc = 0;
> >>> +
> >>> +   if (!dp_panel) {
> >>> +   DRM_ERROR("invalid input\n");
> >>> +   rc = -EINVAL;
> >>> +   return rc;
> >>> +   }
> >>> +
> >>> +   panel = container_of(dp_panel, struct dp_panel_private,
> >>> dp_panel);
> >>> +   catalog = panel->catalog;
> >>> +   dp_mode = _panel->dp_mode;
> >>> +   buff_size = sizeof(gen_buffer);
> >>> +
> >>> +   memset(_header, 0, sizeof(sdp_header));
> >>> +   memset(_sdp_data, 0, sizeof(vsc_sdp_data));
> >>> +
> >>> +   /* VSC SDP header as per table 2-118 of DP 1.4 specification 
> >>> */
> >>> +   sdp_header.HB0 = 0x00;
> >>> +   sdp_header.HB1 = 0x07;
> >>> +   sdp_header.HB2 = 0x05;
> >>> +   sdp_header.HB3 = 0x13;
> >>
> >> This should go to the packing function.
> >>
> >
> > We can  but 
> >
> > the header bytes can change based on the format. These values are
> > specific to YUV420. Thats why this part was kept outside of the generic
> > vsc sdp packing. Today we support only YUV420 VSC SDP so we can move it
> > but this was the reason.
> >>>
> >>> These values can be set from the sdp_type, revision and length fields
> >>> of struct drm_dp_vsc_sdp.
> >>> The function 

Re: [PATCH v2 13/19] drm/msm/dp: add VSC SDP support for YUV420 over DP

2024-02-11 Thread Abhinav Kumar




On 2/10/2024 10:57 PM, Dmitry Baryshkov wrote:

On Sun, 11 Feb 2024 at 06:06, Abhinav Kumar  wrote:




On 2/10/2024 1:46 PM, Dmitry Baryshkov wrote:

On Sat, 10 Feb 2024 at 20:50, Abhinav Kumar  wrote:




On 2/10/2024 10:14 AM, Abhinav Kumar wrote:



On 2/10/2024 2:09 AM, Dmitry Baryshkov wrote:

On Sat, 10 Feb 2024 at 03:52, Paloma Arellano
 wrote:


Add support to pack and send the VSC SDP packet for DP. This therefore
allows the transmision of format information to the sinks which is
needed for YUV420 support over DP.

Changes in v2:
   - Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
   - Remove dp_sdp from the dp_catalog struct since this data is
 being allocated at the point used
   - Create a new function in dp_utils to pack the VSC SDP data
 into a buffer
   - Create a new function that packs the SDP header bytes into a
 buffer. This function is made generic so that it can be
 utilized by dp_audio
 header bytes into a buffer
   - Create a new function in dp_utils that takes the packed
buffer
 and writes to the DP_GENERIC0_* registers
   - Split the dp_catalog_panel_config_vsc_sdp() function into two
 to disable/enable sending VSC SDP packets
   - Check the DP HW version using the original useage of
 dp_catalog_hw_revision() and correct the version checking
 logic
   - Rename dp_panel_setup_vsc_sdp() to
 dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
 currently VSC SDP is only being set up to support YUV420
modes

Signed-off-by: Paloma Arellano 
---
drivers/gpu/drm/msm/dp/dp_catalog.c | 105 
drivers/gpu/drm/msm/dp/dp_catalog.h |   6 ++
drivers/gpu/drm/msm/dp/dp_ctrl.c|   4 ++
drivers/gpu/drm/msm/dp/dp_panel.c   |  59 
drivers/gpu/drm/msm/dp/dp_reg.h |   3 +
drivers/gpu/drm/msm/dp/dp_utils.c   |  80 +
drivers/gpu/drm/msm/dp/dp_utils.h   |   3 +
7 files changed, 260 insertions(+)

diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 5d84c089e520a..0f28a4897b7b7 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -901,6 +901,111 @@ int dp_catalog_panel_timing_cfg(struct
dp_catalog *dp_catalog)
   return 0;
}






+static int dp_panel_setup_vsc_sdp_yuv_420(struct dp_panel *dp_panel)
+{
+   struct dp_catalog *catalog;
+   struct dp_panel_private *panel;
+   struct dp_display_mode *dp_mode;
+   struct dp_sdp_header sdp_header;
+   struct drm_dp_vsc_sdp vsc_sdp_data;
+   size_t buff_size;
+   u32 gen_buffer[10];
+   int rc = 0;
+
+   if (!dp_panel) {
+   DRM_ERROR("invalid input\n");
+   rc = -EINVAL;
+   return rc;
+   }
+
+   panel = container_of(dp_panel, struct dp_panel_private,
dp_panel);
+   catalog = panel->catalog;
+   dp_mode = _panel->dp_mode;
+   buff_size = sizeof(gen_buffer);
+
+   memset(_header, 0, sizeof(sdp_header));
+   memset(_sdp_data, 0, sizeof(vsc_sdp_data));
+
+   /* VSC SDP header as per table 2-118 of DP 1.4 specification */
+   sdp_header.HB0 = 0x00;
+   sdp_header.HB1 = 0x07;
+   sdp_header.HB2 = 0x05;
+   sdp_header.HB3 = 0x13;


This should go to the packing function.



We can  but 

the header bytes can change based on the format. These values are
specific to YUV420. Thats why this part was kept outside of the generic
vsc sdp packing. Today we support only YUV420 VSC SDP so we can move it
but this was the reason.


These values can be set from the sdp_type, revision and length fields
of struct drm_dp_vsc_sdp.
The function intel_dp_vsc_sdp_pack() is pretty much close to what I had in mind.




+
+   /* VSC SDP Payload for DB16 */


Comments are useless more or less. The code fills generic information
structure which might or might not be packed to the data buffer.


+   vsc_sdp_data.pixelformat = DP_PIXELFORMAT_YUV420;
+   vsc_sdp_data.colorimetry = DP_COLORIMETRY_DEFAULT;
+
+   /* VSC SDP Payload for DB17 */
+   vsc_sdp_data.dynamic_range = DP_DYNAMIC_RANGE_CTA;
+
+   /* VSC SDP Payload for DB18 */
+   vsc_sdp_data.content_type = DP_CONTENT_TYPE_GRAPHICS;
+
+   vsc_sdp_data.bpc = dp_mode->bpp / 3;


Consider extracting intel_dp_compute_vsc_colorimetry() and using it.



intel_dp_compute_vsc_colorimetry() uses colorspace property to pick
YUV420, we do not.


Intel function also uses output_format, but it's true, it is full of
Intel specifics.


Right now, its a pure driver decision to use YUV420
when the mode is supported only in that format.

Also, many params are to be used within this function cached inside
intel_crtc_state . We will first need to make that API more generic to
be re-usable by others.


Re: [PATCH v2 13/19] drm/msm/dp: add VSC SDP support for YUV420 over DP

2024-02-10 Thread Dmitry Baryshkov
On Sun, 11 Feb 2024 at 06:06, Abhinav Kumar  wrote:
>
>
>
> On 2/10/2024 1:46 PM, Dmitry Baryshkov wrote:
> > On Sat, 10 Feb 2024 at 20:50, Abhinav Kumar  
> > wrote:
> >>
> >>
> >>
> >> On 2/10/2024 10:14 AM, Abhinav Kumar wrote:
> >>>
> >>>
> >>> On 2/10/2024 2:09 AM, Dmitry Baryshkov wrote:
>  On Sat, 10 Feb 2024 at 03:52, Paloma Arellano
>   wrote:
> >
> > Add support to pack and send the VSC SDP packet for DP. This therefore
> > allows the transmision of format information to the sinks which is
> > needed for YUV420 support over DP.
> >
> > Changes in v2:
> >   - Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
> >   - Remove dp_sdp from the dp_catalog struct since this data is
> > being allocated at the point used
> >   - Create a new function in dp_utils to pack the VSC SDP data
> > into a buffer
> >   - Create a new function that packs the SDP header bytes into a
> > buffer. This function is made generic so that it can be
> > utilized by dp_audio
> > header bytes into a buffer
> >   - Create a new function in dp_utils that takes the packed
> > buffer
> > and writes to the DP_GENERIC0_* registers
> >   - Split the dp_catalog_panel_config_vsc_sdp() function into 
> > two
> > to disable/enable sending VSC SDP packets
> >   - Check the DP HW version using the original useage of
> > dp_catalog_hw_revision() and correct the version checking
> > logic
> >   - Rename dp_panel_setup_vsc_sdp() to
> > dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
> > currently VSC SDP is only being set up to support YUV420
> > modes
> >
> > Signed-off-by: Paloma Arellano 
> > ---
> >drivers/gpu/drm/msm/dp/dp_catalog.c | 105 
> > 
> >drivers/gpu/drm/msm/dp/dp_catalog.h |   6 ++
> >drivers/gpu/drm/msm/dp/dp_ctrl.c|   4 ++
> >drivers/gpu/drm/msm/dp/dp_panel.c   |  59 
> >drivers/gpu/drm/msm/dp/dp_reg.h |   3 +
> >drivers/gpu/drm/msm/dp/dp_utils.c   |  80 +
> >drivers/gpu/drm/msm/dp/dp_utils.h   |   3 +
> >7 files changed, 260 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
> > b/drivers/gpu/drm/msm/dp/dp_catalog.c
> > index 5d84c089e520a..0f28a4897b7b7 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> > @@ -901,6 +901,111 @@ int dp_catalog_panel_timing_cfg(struct
> > dp_catalog *dp_catalog)
> >   return 0;
> >}
> >
>
> 
>
> > +static int dp_panel_setup_vsc_sdp_yuv_420(struct dp_panel *dp_panel)
> > +{
> > +   struct dp_catalog *catalog;
> > +   struct dp_panel_private *panel;
> > +   struct dp_display_mode *dp_mode;
> > +   struct dp_sdp_header sdp_header;
> > +   struct drm_dp_vsc_sdp vsc_sdp_data;
> > +   size_t buff_size;
> > +   u32 gen_buffer[10];
> > +   int rc = 0;
> > +
> > +   if (!dp_panel) {
> > +   DRM_ERROR("invalid input\n");
> > +   rc = -EINVAL;
> > +   return rc;
> > +   }
> > +
> > +   panel = container_of(dp_panel, struct dp_panel_private,
> > dp_panel);
> > +   catalog = panel->catalog;
> > +   dp_mode = _panel->dp_mode;
> > +   buff_size = sizeof(gen_buffer);
> > +
> > +   memset(_header, 0, sizeof(sdp_header));
> > +   memset(_sdp_data, 0, sizeof(vsc_sdp_data));
> > +
> > +   /* VSC SDP header as per table 2-118 of DP 1.4 specification */
> > +   sdp_header.HB0 = 0x00;
> > +   sdp_header.HB1 = 0x07;
> > +   sdp_header.HB2 = 0x05;
> > +   sdp_header.HB3 = 0x13;
> 
>  This should go to the packing function.
> 
> >>>
> >>> We can  but 
> >>>
> >>> the header bytes can change based on the format. These values are
> >>> specific to YUV420. Thats why this part was kept outside of the generic
> >>> vsc sdp packing. Today we support only YUV420 VSC SDP so we can move it
> >>> but this was the reason.
> >
> > These values can be set from the sdp_type, revision and length fields
> > of struct drm_dp_vsc_sdp.
> > The function intel_dp_vsc_sdp_pack() is pretty much close to what I had in 
> > mind.
> >
> >>>
> > +
> > +   /* VSC SDP Payload for DB16 */
> 
>  Comments are useless more or less. The code fills generic information
>  structure which might or might not be packed to the data buffer.
> 
> > +   vsc_sdp_data.pixelformat = DP_PIXELFORMAT_YUV420;
> > +   vsc_sdp_data.colorimetry = DP_COLORIMETRY_DEFAULT;

Re: [PATCH v2 13/19] drm/msm/dp: add VSC SDP support for YUV420 over DP

2024-02-10 Thread Abhinav Kumar




On 2/10/2024 1:46 PM, Dmitry Baryshkov wrote:

On Sat, 10 Feb 2024 at 20:50, Abhinav Kumar  wrote:




On 2/10/2024 10:14 AM, Abhinav Kumar wrote:



On 2/10/2024 2:09 AM, Dmitry Baryshkov wrote:

On Sat, 10 Feb 2024 at 03:52, Paloma Arellano
 wrote:


Add support to pack and send the VSC SDP packet for DP. This therefore
allows the transmision of format information to the sinks which is
needed for YUV420 support over DP.

Changes in v2:
  - Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
  - Remove dp_sdp from the dp_catalog struct since this data is
being allocated at the point used
  - Create a new function in dp_utils to pack the VSC SDP data
into a buffer
  - Create a new function that packs the SDP header bytes into a
buffer. This function is made generic so that it can be
utilized by dp_audio
header bytes into a buffer
  - Create a new function in dp_utils that takes the packed
buffer
and writes to the DP_GENERIC0_* registers
  - Split the dp_catalog_panel_config_vsc_sdp() function into two
to disable/enable sending VSC SDP packets
  - Check the DP HW version using the original useage of
dp_catalog_hw_revision() and correct the version checking
logic
  - Rename dp_panel_setup_vsc_sdp() to
dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
currently VSC SDP is only being set up to support YUV420
modes

Signed-off-by: Paloma Arellano 
---
   drivers/gpu/drm/msm/dp/dp_catalog.c | 105 
   drivers/gpu/drm/msm/dp/dp_catalog.h |   6 ++
   drivers/gpu/drm/msm/dp/dp_ctrl.c|   4 ++
   drivers/gpu/drm/msm/dp/dp_panel.c   |  59 
   drivers/gpu/drm/msm/dp/dp_reg.h |   3 +
   drivers/gpu/drm/msm/dp/dp_utils.c   |  80 +
   drivers/gpu/drm/msm/dp/dp_utils.h   |   3 +
   7 files changed, 260 insertions(+)

diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 5d84c089e520a..0f28a4897b7b7 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -901,6 +901,111 @@ int dp_catalog_panel_timing_cfg(struct
dp_catalog *dp_catalog)
  return 0;
   }






+static int dp_panel_setup_vsc_sdp_yuv_420(struct dp_panel *dp_panel)
+{
+   struct dp_catalog *catalog;
+   struct dp_panel_private *panel;
+   struct dp_display_mode *dp_mode;
+   struct dp_sdp_header sdp_header;
+   struct drm_dp_vsc_sdp vsc_sdp_data;
+   size_t buff_size;
+   u32 gen_buffer[10];
+   int rc = 0;
+
+   if (!dp_panel) {
+   DRM_ERROR("invalid input\n");
+   rc = -EINVAL;
+   return rc;
+   }
+
+   panel = container_of(dp_panel, struct dp_panel_private,
dp_panel);
+   catalog = panel->catalog;
+   dp_mode = _panel->dp_mode;
+   buff_size = sizeof(gen_buffer);
+
+   memset(_header, 0, sizeof(sdp_header));
+   memset(_sdp_data, 0, sizeof(vsc_sdp_data));
+
+   /* VSC SDP header as per table 2-118 of DP 1.4 specification */
+   sdp_header.HB0 = 0x00;
+   sdp_header.HB1 = 0x07;
+   sdp_header.HB2 = 0x05;
+   sdp_header.HB3 = 0x13;


This should go to the packing function.



We can  but 

the header bytes can change based on the format. These values are
specific to YUV420. Thats why this part was kept outside of the generic
vsc sdp packing. Today we support only YUV420 VSC SDP so we can move it
but this was the reason.


These values can be set from the sdp_type, revision and length fields
of struct drm_dp_vsc_sdp.
The function intel_dp_vsc_sdp_pack() is pretty much close to what I had in mind.




+
+   /* VSC SDP Payload for DB16 */


Comments are useless more or less. The code fills generic information
structure which might or might not be packed to the data buffer.


+   vsc_sdp_data.pixelformat = DP_PIXELFORMAT_YUV420;
+   vsc_sdp_data.colorimetry = DP_COLORIMETRY_DEFAULT;
+
+   /* VSC SDP Payload for DB17 */
+   vsc_sdp_data.dynamic_range = DP_DYNAMIC_RANGE_CTA;
+
+   /* VSC SDP Payload for DB18 */
+   vsc_sdp_data.content_type = DP_CONTENT_TYPE_GRAPHICS;
+
+   vsc_sdp_data.bpc = dp_mode->bpp / 3;


Consider extracting intel_dp_compute_vsc_colorimetry() and using it.



intel_dp_compute_vsc_colorimetry() uses colorspace property to pick
YUV420, we do not.


Intel function also uses output_format, but it's true, it is full of
Intel specifics.


Right now, its a pure driver decision to use YUV420
when the mode is supported only in that format.

Also, many params are to be used within this function cached inside
intel_crtc_state . We will first need to make that API more generic to
be re-usable by others.

I think overall, if we want to have a generic packing across vendors, it
needs more work. I think one of us can take that up as a 

Re: [PATCH v2 13/19] drm/msm/dp: add VSC SDP support for YUV420 over DP

2024-02-10 Thread Dmitry Baryshkov
On Sat, 10 Feb 2024 at 20:50, Abhinav Kumar  wrote:
>
>
>
> On 2/10/2024 10:14 AM, Abhinav Kumar wrote:
> >
> >
> > On 2/10/2024 2:09 AM, Dmitry Baryshkov wrote:
> >> On Sat, 10 Feb 2024 at 03:52, Paloma Arellano
> >>  wrote:
> >>>
> >>> Add support to pack and send the VSC SDP packet for DP. This therefore
> >>> allows the transmision of format information to the sinks which is
> >>> needed for YUV420 support over DP.
> >>>
> >>> Changes in v2:
> >>>  - Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
> >>>  - Remove dp_sdp from the dp_catalog struct since this data is
> >>>being allocated at the point used
> >>>  - Create a new function in dp_utils to pack the VSC SDP data
> >>>into a buffer
> >>>  - Create a new function that packs the SDP header bytes into a
> >>>buffer. This function is made generic so that it can be
> >>>utilized by dp_audio
> >>>header bytes into a buffer
> >>>  - Create a new function in dp_utils that takes the packed
> >>> buffer
> >>>and writes to the DP_GENERIC0_* registers
> >>>  - Split the dp_catalog_panel_config_vsc_sdp() function into two
> >>>to disable/enable sending VSC SDP packets
> >>>  - Check the DP HW version using the original useage of
> >>>dp_catalog_hw_revision() and correct the version checking
> >>>logic
> >>>  - Rename dp_panel_setup_vsc_sdp() to
> >>>dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
> >>>currently VSC SDP is only being set up to support YUV420
> >>> modes
> >>>
> >>> Signed-off-by: Paloma Arellano 
> >>> ---
> >>>   drivers/gpu/drm/msm/dp/dp_catalog.c | 105 
> >>>   drivers/gpu/drm/msm/dp/dp_catalog.h |   6 ++
> >>>   drivers/gpu/drm/msm/dp/dp_ctrl.c|   4 ++
> >>>   drivers/gpu/drm/msm/dp/dp_panel.c   |  59 
> >>>   drivers/gpu/drm/msm/dp/dp_reg.h |   3 +
> >>>   drivers/gpu/drm/msm/dp/dp_utils.c   |  80 +
> >>>   drivers/gpu/drm/msm/dp/dp_utils.h   |   3 +
> >>>   7 files changed, 260 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
> >>> b/drivers/gpu/drm/msm/dp/dp_catalog.c
> >>> index 5d84c089e520a..0f28a4897b7b7 100644
> >>> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> >>> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> >>> @@ -901,6 +901,111 @@ int dp_catalog_panel_timing_cfg(struct
> >>> dp_catalog *dp_catalog)
> >>>  return 0;
> >>>   }
> >>>
> >>> +static void dp_catalog_panel_send_vsc_sdp(struct dp_catalog
> >>> *dp_catalog, u32 *buffer)
> >>> +{
> >>> +   struct dp_catalog_private *catalog;
> >>> +
> >>> +   if (!dp_catalog) {
> >>> +   DRM_ERROR("invalid input\n");
> >>> +   return;
> >>> +   }
> >>> +
> >>> +   catalog = container_of(dp_catalog, struct dp_catalog_private,
> >>> dp_catalog);
> >>> +
> >>> +   dp_write_link(catalog, MMSS_DP_GENERIC0_0, buffer[0]);
> >>> +   dp_write_link(catalog, MMSS_DP_GENERIC0_1, buffer[1]);
> >>> +   dp_write_link(catalog, MMSS_DP_GENERIC0_2, buffer[2]);
> >>> +   dp_write_link(catalog, MMSS_DP_GENERIC0_3, buffer[3]);
> >>> +   dp_write_link(catalog, MMSS_DP_GENERIC0_4, buffer[4]);
> >>> +   dp_write_link(catalog, MMSS_DP_GENERIC0_5, buffer[5]);
> >>> +   dp_write_link(catalog, MMSS_DP_GENERIC0_6, buffer[6]);
> >>> +   dp_write_link(catalog, MMSS_DP_GENERIC0_7, buffer[7]);
> >>> +   dp_write_link(catalog, MMSS_DP_GENERIC0_8, buffer[8]);
> >>> +   dp_write_link(catalog, MMSS_DP_GENERIC0_9, buffer[9]);
> >>> +}
> >>> +
> >>> +static void dp_catalog_panel_update_sdp(struct dp_catalog *dp_catalog)
> >>> +{
> >>> +   struct dp_catalog_private *catalog;
> >>> +   u32 hw_revision;
> >>> +
> >>> +   catalog = container_of(dp_catalog, struct dp_catalog_private,
> >>> dp_catalog);
> >>> +
> >>> +   hw_revision = dp_catalog_hw_revision(dp_catalog);
> >>> +   if (hw_revision < DP_HW_VERSION_1_2 && hw_revision >=
> >>> DP_HW_VERSION_1_0) {
> >>> +   dp_write_link(catalog, MMSS_DP_SDP_CFG3, 0x01);
> >>> +   dp_write_link(catalog, MMSS_DP_SDP_CFG3, 0x00);
> >>> +   }
> >>> +}
> >>> +
> >>> +void dp_catalog_panel_enable_vsc_sdp(struct dp_catalog *dp_catalog,
> >>> u32 *gen_buffer)
> >>> +{
> >>> +   struct dp_catalog_private *catalog;
> >>> +   u32 cfg, cfg2, misc;
> >>> +
> >>> +   if (!dp_catalog) {
> >>> +   DRM_ERROR("invalid input\n");
> >>> +   return;
> >>> +   }
> >>> +
> >>> +   catalog = container_of(dp_catalog, struct dp_catalog_private,
> >>> dp_catalog);
> >>> +
> >>> +   cfg = dp_read_link(catalog, MMSS_DP_SDP_CFG);
> >>> +   cfg2 = dp_read_link(catalog, MMSS_DP_SDP_CFG2);
> >>> +   misc = dp_read_link(catalog, REG_DP_MISC1_MISC0);
> >>> +
> >>> +   cfg |= GEN0_SDP_EN;
> >>> +   

Re: [PATCH v2 13/19] drm/msm/dp: add VSC SDP support for YUV420 over DP

2024-02-10 Thread Abhinav Kumar




On 2/10/2024 10:14 AM, Abhinav Kumar wrote:



On 2/10/2024 2:09 AM, Dmitry Baryshkov wrote:
On Sat, 10 Feb 2024 at 03:52, Paloma Arellano 
 wrote:


Add support to pack and send the VSC SDP packet for DP. This therefore
allows the transmision of format information to the sinks which is
needed for YUV420 support over DP.

Changes in v2:
 - Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
 - Remove dp_sdp from the dp_catalog struct since this data is
   being allocated at the point used
 - Create a new function in dp_utils to pack the VSC SDP data
   into a buffer
 - Create a new function that packs the SDP header bytes into a
   buffer. This function is made generic so that it can be
   utilized by dp_audio
   header bytes into a buffer
 - Create a new function in dp_utils that takes the packed 
buffer

   and writes to the DP_GENERIC0_* registers
 - Split the dp_catalog_panel_config_vsc_sdp() function into two
   to disable/enable sending VSC SDP packets
 - Check the DP HW version using the original useage of
   dp_catalog_hw_revision() and correct the version checking
   logic
 - Rename dp_panel_setup_vsc_sdp() to
   dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
   currently VSC SDP is only being set up to support YUV420 
modes


Signed-off-by: Paloma Arellano 
---
  drivers/gpu/drm/msm/dp/dp_catalog.c | 105 
  drivers/gpu/drm/msm/dp/dp_catalog.h |   6 ++
  drivers/gpu/drm/msm/dp/dp_ctrl.c    |   4 ++
  drivers/gpu/drm/msm/dp/dp_panel.c   |  59 
  drivers/gpu/drm/msm/dp/dp_reg.h |   3 +
  drivers/gpu/drm/msm/dp/dp_utils.c   |  80 +
  drivers/gpu/drm/msm/dp/dp_utils.h   |   3 +
  7 files changed, 260 insertions(+)

diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c 
b/drivers/gpu/drm/msm/dp/dp_catalog.c

index 5d84c089e520a..0f28a4897b7b7 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -901,6 +901,111 @@ int dp_catalog_panel_timing_cfg(struct 
dp_catalog *dp_catalog)

 return 0;
  }

+static void dp_catalog_panel_send_vsc_sdp(struct dp_catalog 
*dp_catalog, u32 *buffer)

+{
+   struct dp_catalog_private *catalog;
+
+   if (!dp_catalog) {
+   DRM_ERROR("invalid input\n");
+   return;
+   }
+
+   catalog = container_of(dp_catalog, struct dp_catalog_private, 
dp_catalog);

+
+   dp_write_link(catalog, MMSS_DP_GENERIC0_0, buffer[0]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_1, buffer[1]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_2, buffer[2]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_3, buffer[3]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_4, buffer[4]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_5, buffer[5]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_6, buffer[6]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_7, buffer[7]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_8, buffer[8]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_9, buffer[9]);
+}
+
+static void dp_catalog_panel_update_sdp(struct dp_catalog *dp_catalog)
+{
+   struct dp_catalog_private *catalog;
+   u32 hw_revision;
+
+   catalog = container_of(dp_catalog, struct dp_catalog_private, 
dp_catalog);

+
+   hw_revision = dp_catalog_hw_revision(dp_catalog);
+   if (hw_revision < DP_HW_VERSION_1_2 && hw_revision >= 
DP_HW_VERSION_1_0) {

+   dp_write_link(catalog, MMSS_DP_SDP_CFG3, 0x01);
+   dp_write_link(catalog, MMSS_DP_SDP_CFG3, 0x00);
+   }
+}
+
+void dp_catalog_panel_enable_vsc_sdp(struct dp_catalog *dp_catalog, 
u32 *gen_buffer)

+{
+   struct dp_catalog_private *catalog;
+   u32 cfg, cfg2, misc;
+
+   if (!dp_catalog) {
+   DRM_ERROR("invalid input\n");
+   return;
+   }
+
+   catalog = container_of(dp_catalog, struct dp_catalog_private, 
dp_catalog);

+
+   cfg = dp_read_link(catalog, MMSS_DP_SDP_CFG);
+   cfg2 = dp_read_link(catalog, MMSS_DP_SDP_CFG2);
+   misc = dp_read_link(catalog, REG_DP_MISC1_MISC0);
+
+   cfg |= GEN0_SDP_EN;
+   dp_write_link(catalog, MMSS_DP_SDP_CFG, cfg);
+
+   cfg2 |= GENERIC0_SDPSIZE_VALID;
+   dp_write_link(catalog, MMSS_DP_SDP_CFG2, cfg2);
+
+   dp_catalog_panel_send_vsc_sdp(dp_catalog, gen_buffer);
+
+   /* indicates presence of VSC (BIT(6) of MISC1) */
+   misc |= DP_MISC1_VSC_SDP;
+
+   drm_dbg_dp(catalog->drm_dev, "vsc sdp enable=1\n");
+
+   pr_debug("misc settings = 0x%x\n", misc);
+   dp_write_link(catalog, REG_DP_MISC1_MISC0, misc);
+
+   dp_catalog_panel_update_sdp(dp_catalog);
+}
+
+void dp_catalog_panel_disable_vsc_sdp(struct dp_catalog *dp_catalog)
+{
+   struct dp_catalog_private *catalog;
+   u32 cfg, cfg2, misc;
+
+   if (!dp_catalog) {
+   

Re: [PATCH v2 13/19] drm/msm/dp: add VSC SDP support for YUV420 over DP

2024-02-10 Thread Abhinav Kumar




On 2/10/2024 2:09 AM, Dmitry Baryshkov wrote:

On Sat, 10 Feb 2024 at 03:52, Paloma Arellano  wrote:


Add support to pack and send the VSC SDP packet for DP. This therefore
allows the transmision of format information to the sinks which is
needed for YUV420 support over DP.

Changes in v2:
 - Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
 - Remove dp_sdp from the dp_catalog struct since this data is
   being allocated at the point used
 - Create a new function in dp_utils to pack the VSC SDP data
   into a buffer
 - Create a new function that packs the SDP header bytes into a
   buffer. This function is made generic so that it can be
   utilized by dp_audio
   header bytes into a buffer
 - Create a new function in dp_utils that takes the packed buffer
   and writes to the DP_GENERIC0_* registers
 - Split the dp_catalog_panel_config_vsc_sdp() function into two
   to disable/enable sending VSC SDP packets
 - Check the DP HW version using the original useage of
   dp_catalog_hw_revision() and correct the version checking
   logic
 - Rename dp_panel_setup_vsc_sdp() to
   dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
   currently VSC SDP is only being set up to support YUV420 modes

Signed-off-by: Paloma Arellano 
---
  drivers/gpu/drm/msm/dp/dp_catalog.c | 105 
  drivers/gpu/drm/msm/dp/dp_catalog.h |   6 ++
  drivers/gpu/drm/msm/dp/dp_ctrl.c|   4 ++
  drivers/gpu/drm/msm/dp/dp_panel.c   |  59 
  drivers/gpu/drm/msm/dp/dp_reg.h |   3 +
  drivers/gpu/drm/msm/dp/dp_utils.c   |  80 +
  drivers/gpu/drm/msm/dp/dp_utils.h   |   3 +
  7 files changed, 260 insertions(+)

diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c 
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 5d84c089e520a..0f28a4897b7b7 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -901,6 +901,111 @@ int dp_catalog_panel_timing_cfg(struct dp_catalog 
*dp_catalog)
 return 0;
  }

+static void dp_catalog_panel_send_vsc_sdp(struct dp_catalog *dp_catalog, u32 
*buffer)
+{
+   struct dp_catalog_private *catalog;
+
+   if (!dp_catalog) {
+   DRM_ERROR("invalid input\n");
+   return;
+   }
+
+   catalog = container_of(dp_catalog, struct dp_catalog_private, 
dp_catalog);
+
+   dp_write_link(catalog, MMSS_DP_GENERIC0_0, buffer[0]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_1, buffer[1]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_2, buffer[2]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_3, buffer[3]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_4, buffer[4]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_5, buffer[5]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_6, buffer[6]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_7, buffer[7]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_8, buffer[8]);
+   dp_write_link(catalog, MMSS_DP_GENERIC0_9, buffer[9]);
+}
+
+static void dp_catalog_panel_update_sdp(struct dp_catalog *dp_catalog)
+{
+   struct dp_catalog_private *catalog;
+   u32 hw_revision;
+
+   catalog = container_of(dp_catalog, struct dp_catalog_private, 
dp_catalog);
+
+   hw_revision = dp_catalog_hw_revision(dp_catalog);
+   if (hw_revision < DP_HW_VERSION_1_2 && hw_revision >= 
DP_HW_VERSION_1_0) {
+   dp_write_link(catalog, MMSS_DP_SDP_CFG3, 0x01);
+   dp_write_link(catalog, MMSS_DP_SDP_CFG3, 0x00);
+   }
+}
+
+void dp_catalog_panel_enable_vsc_sdp(struct dp_catalog *dp_catalog, u32 
*gen_buffer)
+{
+   struct dp_catalog_private *catalog;
+   u32 cfg, cfg2, misc;
+
+   if (!dp_catalog) {
+   DRM_ERROR("invalid input\n");
+   return;
+   }
+
+   catalog = container_of(dp_catalog, struct dp_catalog_private, 
dp_catalog);
+
+   cfg = dp_read_link(catalog, MMSS_DP_SDP_CFG);
+   cfg2 = dp_read_link(catalog, MMSS_DP_SDP_CFG2);
+   misc = dp_read_link(catalog, REG_DP_MISC1_MISC0);
+
+   cfg |= GEN0_SDP_EN;
+   dp_write_link(catalog, MMSS_DP_SDP_CFG, cfg);
+
+   cfg2 |= GENERIC0_SDPSIZE_VALID;
+   dp_write_link(catalog, MMSS_DP_SDP_CFG2, cfg2);
+
+   dp_catalog_panel_send_vsc_sdp(dp_catalog, gen_buffer);
+
+   /* indicates presence of VSC (BIT(6) of MISC1) */
+   misc |= DP_MISC1_VSC_SDP;
+
+   drm_dbg_dp(catalog->drm_dev, "vsc sdp enable=1\n");
+
+   pr_debug("misc settings = 0x%x\n", misc);
+   dp_write_link(catalog, REG_DP_MISC1_MISC0, misc);
+
+   dp_catalog_panel_update_sdp(dp_catalog);
+}
+
+void dp_catalog_panel_disable_vsc_sdp(struct dp_catalog *dp_catalog)
+{
+   struct dp_catalog_private *catalog;
+   u32 cfg, cfg2, misc;
+
+   if (!dp_catalog) {
+   DRM_ERROR("invalid input\n");
+   return;
+   }
+

Re: [PATCH v2 13/19] drm/msm/dp: add VSC SDP support for YUV420 over DP

2024-02-10 Thread Dmitry Baryshkov
On Sat, 10 Feb 2024 at 03:52, Paloma Arellano  wrote:
>
> Add support to pack and send the VSC SDP packet for DP. This therefore
> allows the transmision of format information to the sinks which is
> needed for YUV420 support over DP.
>
> Changes in v2:
> - Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
> - Remove dp_sdp from the dp_catalog struct since this data is
>   being allocated at the point used
> - Create a new function in dp_utils to pack the VSC SDP data
>   into a buffer
> - Create a new function that packs the SDP header bytes into a
>   buffer. This function is made generic so that it can be
>   utilized by dp_audio
>   header bytes into a buffer
> - Create a new function in dp_utils that takes the packed buffer
>   and writes to the DP_GENERIC0_* registers
> - Split the dp_catalog_panel_config_vsc_sdp() function into two
>   to disable/enable sending VSC SDP packets
> - Check the DP HW version using the original useage of
>   dp_catalog_hw_revision() and correct the version checking
>   logic
> - Rename dp_panel_setup_vsc_sdp() to
>   dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
>   currently VSC SDP is only being set up to support YUV420 modes
>
> Signed-off-by: Paloma Arellano 
> ---
>  drivers/gpu/drm/msm/dp/dp_catalog.c | 105 
>  drivers/gpu/drm/msm/dp/dp_catalog.h |   6 ++
>  drivers/gpu/drm/msm/dp/dp_ctrl.c|   4 ++
>  drivers/gpu/drm/msm/dp/dp_panel.c   |  59 
>  drivers/gpu/drm/msm/dp/dp_reg.h |   3 +
>  drivers/gpu/drm/msm/dp/dp_utils.c   |  80 +
>  drivers/gpu/drm/msm/dp/dp_utils.h   |   3 +
>  7 files changed, 260 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c 
> b/drivers/gpu/drm/msm/dp/dp_catalog.c
> index 5d84c089e520a..0f28a4897b7b7 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> @@ -901,6 +901,111 @@ int dp_catalog_panel_timing_cfg(struct dp_catalog 
> *dp_catalog)
> return 0;
>  }
>
> +static void dp_catalog_panel_send_vsc_sdp(struct dp_catalog *dp_catalog, u32 
> *buffer)
> +{
> +   struct dp_catalog_private *catalog;
> +
> +   if (!dp_catalog) {
> +   DRM_ERROR("invalid input\n");
> +   return;
> +   }
> +
> +   catalog = container_of(dp_catalog, struct dp_catalog_private, 
> dp_catalog);
> +
> +   dp_write_link(catalog, MMSS_DP_GENERIC0_0, buffer[0]);
> +   dp_write_link(catalog, MMSS_DP_GENERIC0_1, buffer[1]);
> +   dp_write_link(catalog, MMSS_DP_GENERIC0_2, buffer[2]);
> +   dp_write_link(catalog, MMSS_DP_GENERIC0_3, buffer[3]);
> +   dp_write_link(catalog, MMSS_DP_GENERIC0_4, buffer[4]);
> +   dp_write_link(catalog, MMSS_DP_GENERIC0_5, buffer[5]);
> +   dp_write_link(catalog, MMSS_DP_GENERIC0_6, buffer[6]);
> +   dp_write_link(catalog, MMSS_DP_GENERIC0_7, buffer[7]);
> +   dp_write_link(catalog, MMSS_DP_GENERIC0_8, buffer[8]);
> +   dp_write_link(catalog, MMSS_DP_GENERIC0_9, buffer[9]);
> +}
> +
> +static void dp_catalog_panel_update_sdp(struct dp_catalog *dp_catalog)
> +{
> +   struct dp_catalog_private *catalog;
> +   u32 hw_revision;
> +
> +   catalog = container_of(dp_catalog, struct dp_catalog_private, 
> dp_catalog);
> +
> +   hw_revision = dp_catalog_hw_revision(dp_catalog);
> +   if (hw_revision < DP_HW_VERSION_1_2 && hw_revision >= 
> DP_HW_VERSION_1_0) {
> +   dp_write_link(catalog, MMSS_DP_SDP_CFG3, 0x01);
> +   dp_write_link(catalog, MMSS_DP_SDP_CFG3, 0x00);
> +   }
> +}
> +
> +void dp_catalog_panel_enable_vsc_sdp(struct dp_catalog *dp_catalog, u32 
> *gen_buffer)
> +{
> +   struct dp_catalog_private *catalog;
> +   u32 cfg, cfg2, misc;
> +
> +   if (!dp_catalog) {
> +   DRM_ERROR("invalid input\n");
> +   return;
> +   }
> +
> +   catalog = container_of(dp_catalog, struct dp_catalog_private, 
> dp_catalog);
> +
> +   cfg = dp_read_link(catalog, MMSS_DP_SDP_CFG);
> +   cfg2 = dp_read_link(catalog, MMSS_DP_SDP_CFG2);
> +   misc = dp_read_link(catalog, REG_DP_MISC1_MISC0);
> +
> +   cfg |= GEN0_SDP_EN;
> +   dp_write_link(catalog, MMSS_DP_SDP_CFG, cfg);
> +
> +   cfg2 |= GENERIC0_SDPSIZE_VALID;
> +   dp_write_link(catalog, MMSS_DP_SDP_CFG2, cfg2);
> +
> +   dp_catalog_panel_send_vsc_sdp(dp_catalog, gen_buffer);
> +
> +   /* indicates presence of VSC (BIT(6) of MISC1) */
> +   misc |= DP_MISC1_VSC_SDP;
> +
> +   drm_dbg_dp(catalog->drm_dev, "vsc sdp enable=1\n");
> +
> +   pr_debug("misc settings = 0x%x\n", misc);
> +   dp_write_link(catalog, REG_DP_MISC1_MISC0, misc);
> +
> +   dp_catalog_panel_update_sdp(dp_catalog);
> +}
> +
> +void dp_catalog_panel_disable_vsc_sdp(struct dp_catalog *dp_catalog)
> +{
> +   struct